* [PATCH v2 00/16] nvme: refactoring and cleanups
@ 2020-04-15 13:01 Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 01/16] nvme: fix pci doorbell size calculation Klaus Jensen
` (19 more replies)
0 siblings, 20 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Changes since v1
~~~~~~~~~~~~~~~~
* nvme: fix pci doorbell size calculation
- added some defines and a better comment (Philippe)
* nvme: rename trace events to pci_nvme
- changed the prefix from nvme_dev to pci_nvme (Philippe)
* nvme: add max_ioqpairs device parameter
- added a deprecation comment. I doubt this will go in until 5.1, so
changed it to "deprecated from 5.1" (Philippe)
* nvme: factor out property/constraint checks
* nvme: factor out block backend setup
- changed to return void and propagate errors in proper QEMU style
(Philippe)
* nvme: add namespace helpers
- use the helper immediately (Philippe)
* nvme: factor out pci setup
- removed setting of vendor and device id which is already inherited
from nvme_class_init() (Philippe)
* nvme: factor out cmb setup
- add lost comment (Philippe)
Klaus Jensen (16):
nvme: fix pci doorbell size calculation
nvme: rename trace events to pci_nvme
nvme: remove superfluous breaks
nvme: move device parameters to separate struct
nvme: use constants in identify
nvme: refactor nvme_addr_read
nvme: add max_ioqpairs device parameter
nvme: remove redundant cmbloc/cmbsz members
nvme: factor out property/constraint checks
nvme: factor out device state setup
nvme: factor out block backend setup
nvme: add namespace helpers
nvme: factor out namespace setup
nvme: factor out pci setup
nvme: factor out cmb setup
nvme: factor out controller identify setup
hw/block/nvme.c | 433 ++++++++++++++++++++++++------------------
hw/block/nvme.h | 36 +++-
hw/block/trace-events | 172 ++++++++---------
include/block/nvme.h | 8 +
4 files changed, 372 insertions(+), 277 deletions(-)
--
2.26.0
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 01/16] nvme: fix pci doorbell size calculation
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:13 ` Philippe Mathieu-Daudé
2020-04-15 13:01 ` [PATCH v2 02/16] nvme: rename trace events to pci_nvme Klaus Jensen
` (18 subsequent siblings)
19 siblings, 1 reply; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
The size of the BAR is 0x1000 (main registers) + 8 bytes for each
queue. Currently, the size of the BAR is calculated like so:
n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
Since the 'num_queues' parameter already accounts for the admin queue,
this should in any case not need to be incremented by one. Also, the
size should be initialized to (0x1000).
n->reg_size = pow2ceil(0x1000 + 2 * n->num_queues * 4);
This, with the default value of num_queues (64), we will set aside room
for 1 admin queue and 63 I/O queues (4 bytes per doorbell, 2 doorbells
per queue).
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d28335cbf377..5b5f75c9d29e 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -43,6 +43,9 @@
#include "trace.h"
#include "nvme.h"
+#define NVME_REG_SIZE 0x1000
+#define NVME_DB_SIZE 4
+
#define NVME_GUEST_ERR(trace, fmt, ...) \
do { \
(trace_##trace)(__VA_ARGS__); \
@@ -1345,7 +1348,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
pcie_endpoint_cap_init(pci_dev, 0x80);
n->num_namespaces = 1;
- n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
+
+ /* num_queues is really number of pairs, so each has two doorbells */
+ n->reg_size = pow2ceil(NVME_REG_SIZE + 2 * n->num_queues * NVME_DB_SIZE);
n->ns_size = bs_size / (uint64_t)n->num_namespaces;
n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 02/16] nvme: rename trace events to pci_nvme
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 01/16] nvme: fix pci doorbell size calculation Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:04 ` Philippe Mathieu-Daudé
2020-04-21 9:43 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 03/16] nvme: remove superfluous breaks Klaus Jensen
` (17 subsequent siblings)
19 siblings, 2 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Change the prefix of all nvme device related trace events to 'pci_nvme'
to not clash with trace events from the nvme block driver.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 190 +++++++++++++++++++++---------------------
hw/block/trace-events | 172 +++++++++++++++++++-------------------
2 files changed, 180 insertions(+), 182 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5b5f75c9d29e..931ddeb26ba0 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -115,16 +115,16 @@ static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
{
if (cq->irq_enabled) {
if (msix_enabled(&(n->parent_obj))) {
- trace_nvme_irq_msix(cq->vector);
+ trace_pci_nvme_irq_msix(cq->vector);
msix_notify(&(n->parent_obj), cq->vector);
} else {
- trace_nvme_irq_pin();
+ trace_pci_nvme_irq_pin();
assert(cq->cqid < 64);
n->irq_status |= 1 << cq->cqid;
nvme_irq_check(n);
}
} else {
- trace_nvme_irq_masked();
+ trace_pci_nvme_irq_masked();
}
}
@@ -149,7 +149,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
int num_prps = (len >> n->page_bits) + 1;
if (unlikely(!prp1)) {
- trace_nvme_err_invalid_prp();
+ trace_pci_nvme_err_invalid_prp();
return NVME_INVALID_FIELD | NVME_DNR;
} else if (n->cmbsz && prp1 >= n->ctrl_mem.addr &&
prp1 < n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size)) {
@@ -163,7 +163,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
len -= trans_len;
if (len) {
if (unlikely(!prp2)) {
- trace_nvme_err_invalid_prp2_missing();
+ trace_pci_nvme_err_invalid_prp2_missing();
goto unmap;
}
if (len > n->page_size) {
@@ -179,7 +179,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
if (i == n->max_prp_ents - 1 && len > n->page_size) {
if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
- trace_nvme_err_invalid_prplist_ent(prp_ent);
+ trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
goto unmap;
}
@@ -192,7 +192,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
}
if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
- trace_nvme_err_invalid_prplist_ent(prp_ent);
+ trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
goto unmap;
}
@@ -207,7 +207,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
}
} else {
if (unlikely(prp2 & (n->page_size - 1))) {
- trace_nvme_err_invalid_prp2_align(prp2);
+ trace_pci_nvme_err_invalid_prp2_align(prp2);
goto unmap;
}
if (qsg->nsg) {
@@ -255,20 +255,20 @@ static uint16_t nvme_dma_read_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len,
QEMUIOVector iov;
uint16_t status = NVME_SUCCESS;
- trace_nvme_dma_read(prp1, prp2);
+ trace_pci_nvme_dma_read(prp1, prp2);
if (nvme_map_prp(&qsg, &iov, prp1, prp2, len, n)) {
return NVME_INVALID_FIELD | NVME_DNR;
}
if (qsg.nsg > 0) {
if (unlikely(dma_buf_read(ptr, len, &qsg))) {
- trace_nvme_err_invalid_dma();
+ trace_pci_nvme_err_invalid_dma();
status = NVME_INVALID_FIELD | NVME_DNR;
}
qemu_sglist_destroy(&qsg);
} else {
if (unlikely(qemu_iovec_from_buf(&iov, 0, ptr, len) != len)) {
- trace_nvme_err_invalid_dma();
+ trace_pci_nvme_err_invalid_dma();
status = NVME_INVALID_FIELD | NVME_DNR;
}
qemu_iovec_destroy(&iov);
@@ -357,7 +357,7 @@ static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd,
uint32_t count = nlb << data_shift;
if (unlikely(slba + nlb > ns->id_ns.nsze)) {
- trace_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
+ trace_pci_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
return NVME_LBA_RANGE | NVME_DNR;
}
@@ -385,11 +385,11 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd,
int is_write = rw->opcode == NVME_CMD_WRITE ? 1 : 0;
enum BlockAcctType acct = is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ;
- trace_nvme_rw(is_write ? "write" : "read", nlb, data_size, slba);
+ trace_pci_nvme_rw(is_write ? "write" : "read", nlb, data_size, slba);
if (unlikely((slba + nlb) > ns->id_ns.nsze)) {
block_acct_invalid(blk_get_stats(n->conf.blk), acct);
- trace_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
+ trace_pci_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
return NVME_LBA_RANGE | NVME_DNR;
}
@@ -424,7 +424,7 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
uint32_t nsid = le32_to_cpu(cmd->nsid);
if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
- trace_nvme_err_invalid_ns(nsid, n->num_namespaces);
+ trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
return NVME_INVALID_NSID | NVME_DNR;
}
@@ -438,7 +438,7 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
case NVME_CMD_READ:
return nvme_rw(n, ns, cmd, req);
default:
- trace_nvme_err_invalid_opc(cmd->opcode);
+ trace_pci_nvme_err_invalid_opc(cmd->opcode);
return NVME_INVALID_OPCODE | NVME_DNR;
}
}
@@ -463,11 +463,11 @@ static uint16_t nvme_del_sq(NvmeCtrl *n, NvmeCmd *cmd)
uint16_t qid = le16_to_cpu(c->qid);
if (unlikely(!qid || nvme_check_sqid(n, qid))) {
- trace_nvme_err_invalid_del_sq(qid);
+ trace_pci_nvme_err_invalid_del_sq(qid);
return NVME_INVALID_QID | NVME_DNR;
}
- trace_nvme_del_sq(qid);
+ trace_pci_nvme_del_sq(qid);
sq = n->sq[qid];
while (!QTAILQ_EMPTY(&sq->out_req_list)) {
@@ -531,26 +531,26 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeCmd *cmd)
uint16_t qflags = le16_to_cpu(c->sq_flags);
uint64_t prp1 = le64_to_cpu(c->prp1);
- trace_nvme_create_sq(prp1, sqid, cqid, qsize, qflags);
+ trace_pci_nvme_create_sq(prp1, sqid, cqid, qsize, qflags);
if (unlikely(!cqid || nvme_check_cqid(n, cqid))) {
- trace_nvme_err_invalid_create_sq_cqid(cqid);
+ trace_pci_nvme_err_invalid_create_sq_cqid(cqid);
return NVME_INVALID_CQID | NVME_DNR;
}
if (unlikely(!sqid || !nvme_check_sqid(n, sqid))) {
- trace_nvme_err_invalid_create_sq_sqid(sqid);
+ trace_pci_nvme_err_invalid_create_sq_sqid(sqid);
return NVME_INVALID_QID | NVME_DNR;
}
if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
- trace_nvme_err_invalid_create_sq_size(qsize);
+ trace_pci_nvme_err_invalid_create_sq_size(qsize);
return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
}
if (unlikely(!prp1 || prp1 & (n->page_size - 1))) {
- trace_nvme_err_invalid_create_sq_addr(prp1);
+ trace_pci_nvme_err_invalid_create_sq_addr(prp1);
return NVME_INVALID_FIELD | NVME_DNR;
}
if (unlikely(!(NVME_SQ_FLAGS_PC(qflags)))) {
- trace_nvme_err_invalid_create_sq_qflags(NVME_SQ_FLAGS_PC(qflags));
+ trace_pci_nvme_err_invalid_create_sq_qflags(NVME_SQ_FLAGS_PC(qflags));
return NVME_INVALID_FIELD | NVME_DNR;
}
sq = g_malloc0(sizeof(*sq));
@@ -576,17 +576,17 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeCmd *cmd)
uint16_t qid = le16_to_cpu(c->qid);
if (unlikely(!qid || nvme_check_cqid(n, qid))) {
- trace_nvme_err_invalid_del_cq_cqid(qid);
+ trace_pci_nvme_err_invalid_del_cq_cqid(qid);
return NVME_INVALID_CQID | NVME_DNR;
}
cq = n->cq[qid];
if (unlikely(!QTAILQ_EMPTY(&cq->sq_list))) {
- trace_nvme_err_invalid_del_cq_notempty(qid);
+ trace_pci_nvme_err_invalid_del_cq_notempty(qid);
return NVME_INVALID_QUEUE_DEL;
}
nvme_irq_deassert(n, cq);
- trace_nvme_del_cq(qid);
+ trace_pci_nvme_del_cq(qid);
nvme_free_cq(cq, n);
return NVME_SUCCESS;
}
@@ -619,27 +619,27 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
uint16_t qflags = le16_to_cpu(c->cq_flags);
uint64_t prp1 = le64_to_cpu(c->prp1);
- trace_nvme_create_cq(prp1, cqid, vector, qsize, qflags,
- NVME_CQ_FLAGS_IEN(qflags) != 0);
+ trace_pci_nvme_create_cq(prp1, cqid, vector, qsize, qflags,
+ NVME_CQ_FLAGS_IEN(qflags) != 0);
if (unlikely(!cqid || !nvme_check_cqid(n, cqid))) {
- trace_nvme_err_invalid_create_cq_cqid(cqid);
+ trace_pci_nvme_err_invalid_create_cq_cqid(cqid);
return NVME_INVALID_CQID | NVME_DNR;
}
if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
- trace_nvme_err_invalid_create_cq_size(qsize);
+ trace_pci_nvme_err_invalid_create_cq_size(qsize);
return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
}
if (unlikely(!prp1)) {
- trace_nvme_err_invalid_create_cq_addr(prp1);
+ trace_pci_nvme_err_invalid_create_cq_addr(prp1);
return NVME_INVALID_FIELD | NVME_DNR;
}
if (unlikely(vector > n->num_queues)) {
- trace_nvme_err_invalid_create_cq_vector(vector);
+ trace_pci_nvme_err_invalid_create_cq_vector(vector);
return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
}
if (unlikely(!(NVME_CQ_FLAGS_PC(qflags)))) {
- trace_nvme_err_invalid_create_cq_qflags(NVME_CQ_FLAGS_PC(qflags));
+ trace_pci_nvme_err_invalid_create_cq_qflags(NVME_CQ_FLAGS_PC(qflags));
return NVME_INVALID_FIELD | NVME_DNR;
}
@@ -654,7 +654,7 @@ static uint16_t nvme_identify_ctrl(NvmeCtrl *n, NvmeIdentify *c)
uint64_t prp1 = le64_to_cpu(c->prp1);
uint64_t prp2 = le64_to_cpu(c->prp2);
- trace_nvme_identify_ctrl();
+ trace_pci_nvme_identify_ctrl();
return nvme_dma_read_prp(n, (uint8_t *)&n->id_ctrl, sizeof(n->id_ctrl),
prp1, prp2);
@@ -667,10 +667,10 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeIdentify *c)
uint64_t prp1 = le64_to_cpu(c->prp1);
uint64_t prp2 = le64_to_cpu(c->prp2);
- trace_nvme_identify_ns(nsid);
+ trace_pci_nvme_identify_ns(nsid);
if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
- trace_nvme_err_invalid_ns(nsid, n->num_namespaces);
+ trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
return NVME_INVALID_NSID | NVME_DNR;
}
@@ -690,7 +690,7 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeIdentify *c)
uint16_t ret;
int i, j = 0;
- trace_nvme_identify_nslist(min_nsid);
+ trace_pci_nvme_identify_nslist(min_nsid);
list = g_malloc0(data_len);
for (i = 0; i < n->num_namespaces; i++) {
@@ -719,14 +719,14 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
case 0x02:
return nvme_identify_nslist(n, c);
default:
- trace_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
+ trace_pci_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
return NVME_INVALID_FIELD | NVME_DNR;
}
}
static inline void nvme_set_timestamp(NvmeCtrl *n, uint64_t ts)
{
- trace_nvme_setfeat_timestamp(ts);
+ trace_pci_nvme_setfeat_timestamp(ts);
n->host_timestamp = le64_to_cpu(ts);
n->timestamp_set_qemu_clock_ms = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
@@ -759,7 +759,7 @@ static inline uint64_t nvme_get_timestamp(const NvmeCtrl *n)
/* If the host timestamp is non-zero, set the timestamp origin */
ts.origin = n->host_timestamp ? 0x01 : 0x00;
- trace_nvme_getfeat_timestamp(ts.all);
+ trace_pci_nvme_getfeat_timestamp(ts.all);
return cpu_to_le64(ts.all);
}
@@ -783,17 +783,17 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
switch (dw10) {
case NVME_VOLATILE_WRITE_CACHE:
result = blk_enable_write_cache(n->conf.blk);
- trace_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
+ trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
break;
case NVME_NUMBER_OF_QUEUES:
result = cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
- trace_nvme_getfeat_numq(result);
+ trace_pci_nvme_getfeat_numq(result);
break;
case NVME_TIMESTAMP:
return nvme_get_feature_timestamp(n, cmd);
break;
default:
- trace_nvme_err_invalid_getfeat(dw10);
+ trace_pci_nvme_err_invalid_getfeat(dw10);
return NVME_INVALID_FIELD | NVME_DNR;
}
@@ -829,9 +829,9 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
blk_set_enable_write_cache(n->conf.blk, dw11 & 1);
break;
case NVME_NUMBER_OF_QUEUES:
- trace_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
- ((dw11 >> 16) & 0xFFFF) + 1,
- n->num_queues - 1, n->num_queues - 1);
+ trace_pci_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
+ ((dw11 >> 16) & 0xFFFF) + 1,
+ n->num_queues - 1, n->num_queues - 1);
req->cqe.result =
cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
break;
@@ -841,7 +841,7 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
break;
default:
- trace_nvme_err_invalid_setfeat(dw10);
+ trace_pci_nvme_err_invalid_setfeat(dw10);
return NVME_INVALID_FIELD | NVME_DNR;
}
return NVME_SUCCESS;
@@ -865,7 +865,7 @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
case NVME_ADM_CMD_GET_FEATURES:
return nvme_get_feature(n, cmd, req);
default:
- trace_nvme_err_invalid_admin_opc(cmd->opcode);
+ trace_pci_nvme_err_invalid_admin_opc(cmd->opcode);
return NVME_INVALID_OPCODE | NVME_DNR;
}
}
@@ -928,77 +928,77 @@ static int nvme_start_ctrl(NvmeCtrl *n)
uint32_t page_size = 1 << page_bits;
if (unlikely(n->cq[0])) {
- trace_nvme_err_startfail_cq();
+ trace_pci_nvme_err_startfail_cq();
return -1;
}
if (unlikely(n->sq[0])) {
- trace_nvme_err_startfail_sq();
+ trace_pci_nvme_err_startfail_sq();
return -1;
}
if (unlikely(!n->bar.asq)) {
- trace_nvme_err_startfail_nbarasq();
+ trace_pci_nvme_err_startfail_nbarasq();
return -1;
}
if (unlikely(!n->bar.acq)) {
- trace_nvme_err_startfail_nbaracq();
+ trace_pci_nvme_err_startfail_nbaracq();
return -1;
}
if (unlikely(n->bar.asq & (page_size - 1))) {
- trace_nvme_err_startfail_asq_misaligned(n->bar.asq);
+ trace_pci_nvme_err_startfail_asq_misaligned(n->bar.asq);
return -1;
}
if (unlikely(n->bar.acq & (page_size - 1))) {
- trace_nvme_err_startfail_acq_misaligned(n->bar.acq);
+ trace_pci_nvme_err_startfail_acq_misaligned(n->bar.acq);
return -1;
}
if (unlikely(NVME_CC_MPS(n->bar.cc) <
NVME_CAP_MPSMIN(n->bar.cap))) {
- trace_nvme_err_startfail_page_too_small(
+ trace_pci_nvme_err_startfail_page_too_small(
NVME_CC_MPS(n->bar.cc),
NVME_CAP_MPSMIN(n->bar.cap));
return -1;
}
if (unlikely(NVME_CC_MPS(n->bar.cc) >
NVME_CAP_MPSMAX(n->bar.cap))) {
- trace_nvme_err_startfail_page_too_large(
+ trace_pci_nvme_err_startfail_page_too_large(
NVME_CC_MPS(n->bar.cc),
NVME_CAP_MPSMAX(n->bar.cap));
return -1;
}
if (unlikely(NVME_CC_IOCQES(n->bar.cc) <
NVME_CTRL_CQES_MIN(n->id_ctrl.cqes))) {
- trace_nvme_err_startfail_cqent_too_small(
+ trace_pci_nvme_err_startfail_cqent_too_small(
NVME_CC_IOCQES(n->bar.cc),
NVME_CTRL_CQES_MIN(n->bar.cap));
return -1;
}
if (unlikely(NVME_CC_IOCQES(n->bar.cc) >
NVME_CTRL_CQES_MAX(n->id_ctrl.cqes))) {
- trace_nvme_err_startfail_cqent_too_large(
+ trace_pci_nvme_err_startfail_cqent_too_large(
NVME_CC_IOCQES(n->bar.cc),
NVME_CTRL_CQES_MAX(n->bar.cap));
return -1;
}
if (unlikely(NVME_CC_IOSQES(n->bar.cc) <
NVME_CTRL_SQES_MIN(n->id_ctrl.sqes))) {
- trace_nvme_err_startfail_sqent_too_small(
+ trace_pci_nvme_err_startfail_sqent_too_small(
NVME_CC_IOSQES(n->bar.cc),
NVME_CTRL_SQES_MIN(n->bar.cap));
return -1;
}
if (unlikely(NVME_CC_IOSQES(n->bar.cc) >
NVME_CTRL_SQES_MAX(n->id_ctrl.sqes))) {
- trace_nvme_err_startfail_sqent_too_large(
+ trace_pci_nvme_err_startfail_sqent_too_large(
NVME_CC_IOSQES(n->bar.cc),
NVME_CTRL_SQES_MAX(n->bar.cap));
return -1;
}
if (unlikely(!NVME_AQA_ASQS(n->bar.aqa))) {
- trace_nvme_err_startfail_asqent_sz_zero();
+ trace_pci_nvme_err_startfail_asqent_sz_zero();
return -1;
}
if (unlikely(!NVME_AQA_ACQS(n->bar.aqa))) {
- trace_nvme_err_startfail_acqent_sz_zero();
+ trace_pci_nvme_err_startfail_acqent_sz_zero();
return -1;
}
@@ -1021,14 +1021,14 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
unsigned size)
{
if (unlikely(offset & (sizeof(uint32_t) - 1))) {
- NVME_GUEST_ERR(nvme_ub_mmiowr_misaligned32,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_misaligned32,
"MMIO write not 32-bit aligned,"
" offset=0x%"PRIx64"", offset);
/* should be ignored, fall through for now */
}
if (unlikely(size < sizeof(uint32_t))) {
- NVME_GUEST_ERR(nvme_ub_mmiowr_toosmall,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_toosmall,
"MMIO write smaller than 32-bits,"
" offset=0x%"PRIx64", size=%u",
offset, size);
@@ -1038,32 +1038,30 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
switch (offset) {
case 0xc: /* INTMS */
if (unlikely(msix_enabled(&(n->parent_obj)))) {
- NVME_GUEST_ERR(nvme_ub_mmiowr_intmask_with_msix,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_intmask_with_msix,
"undefined access to interrupt mask set"
" when MSI-X is enabled");
/* should be ignored, fall through for now */
}
n->bar.intms |= data & 0xffffffff;
n->bar.intmc = n->bar.intms;
- trace_nvme_mmio_intm_set(data & 0xffffffff,
- n->bar.intmc);
+ trace_pci_nvme_mmio_intm_set(data & 0xffffffff, n->bar.intmc);
nvme_irq_check(n);
break;
case 0x10: /* INTMC */
if (unlikely(msix_enabled(&(n->parent_obj)))) {
- NVME_GUEST_ERR(nvme_ub_mmiowr_intmask_with_msix,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_intmask_with_msix,
"undefined access to interrupt mask clr"
" when MSI-X is enabled");
/* should be ignored, fall through for now */
}
n->bar.intms &= ~(data & 0xffffffff);
n->bar.intmc = n->bar.intms;
- trace_nvme_mmio_intm_clr(data & 0xffffffff,
- n->bar.intmc);
+ trace_pci_nvme_mmio_intm_clr(data & 0xffffffff, n->bar.intmc);
nvme_irq_check(n);
break;
case 0x14: /* CC */
- trace_nvme_mmio_cfg(data & 0xffffffff);
+ trace_pci_nvme_mmio_cfg(data & 0xffffffff);
/* Windows first sends data, then sends enable bit */
if (!NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc) &&
!NVME_CC_SHN(data) && !NVME_CC_SHN(n->bar.cc))
@@ -1074,42 +1072,42 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
if (NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc)) {
n->bar.cc = data;
if (unlikely(nvme_start_ctrl(n))) {
- trace_nvme_err_startfail();
+ trace_pci_nvme_err_startfail();
n->bar.csts = NVME_CSTS_FAILED;
} else {
- trace_nvme_mmio_start_success();
+ trace_pci_nvme_mmio_start_success();
n->bar.csts = NVME_CSTS_READY;
}
} else if (!NVME_CC_EN(data) && NVME_CC_EN(n->bar.cc)) {
- trace_nvme_mmio_stopped();
+ trace_pci_nvme_mmio_stopped();
nvme_clear_ctrl(n);
n->bar.csts &= ~NVME_CSTS_READY;
}
if (NVME_CC_SHN(data) && !(NVME_CC_SHN(n->bar.cc))) {
- trace_nvme_mmio_shutdown_set();
+ trace_pci_nvme_mmio_shutdown_set();
nvme_clear_ctrl(n);
n->bar.cc = data;
n->bar.csts |= NVME_CSTS_SHST_COMPLETE;
} else if (!NVME_CC_SHN(data) && NVME_CC_SHN(n->bar.cc)) {
- trace_nvme_mmio_shutdown_cleared();
+ trace_pci_nvme_mmio_shutdown_cleared();
n->bar.csts &= ~NVME_CSTS_SHST_COMPLETE;
n->bar.cc = data;
}
break;
case 0x1C: /* CSTS */
if (data & (1 << 4)) {
- NVME_GUEST_ERR(nvme_ub_mmiowr_ssreset_w1c_unsupported,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_ssreset_w1c_unsupported,
"attempted to W1C CSTS.NSSRO"
" but CAP.NSSRS is zero (not supported)");
} else if (data != 0) {
- NVME_GUEST_ERR(nvme_ub_mmiowr_ro_csts,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_ro_csts,
"attempted to set a read only bit"
" of controller status");
}
break;
case 0x20: /* NSSR */
if (data == 0x4E564D65) {
- trace_nvme_ub_mmiowr_ssreset_unsupported();
+ trace_pci_nvme_ub_mmiowr_ssreset_unsupported();
} else {
/* The spec says that writes of other values have no effect */
return;
@@ -1117,35 +1115,35 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
break;
case 0x24: /* AQA */
n->bar.aqa = data & 0xffffffff;
- trace_nvme_mmio_aqattr(data & 0xffffffff);
+ trace_pci_nvme_mmio_aqattr(data & 0xffffffff);
break;
case 0x28: /* ASQ */
n->bar.asq = data;
- trace_nvme_mmio_asqaddr(data);
+ trace_pci_nvme_mmio_asqaddr(data);
break;
case 0x2c: /* ASQ hi */
n->bar.asq |= data << 32;
- trace_nvme_mmio_asqaddr_hi(data, n->bar.asq);
+ trace_pci_nvme_mmio_asqaddr_hi(data, n->bar.asq);
break;
case 0x30: /* ACQ */
- trace_nvme_mmio_acqaddr(data);
+ trace_pci_nvme_mmio_acqaddr(data);
n->bar.acq = data;
break;
case 0x34: /* ACQ hi */
n->bar.acq |= data << 32;
- trace_nvme_mmio_acqaddr_hi(data, n->bar.acq);
+ trace_pci_nvme_mmio_acqaddr_hi(data, n->bar.acq);
break;
case 0x38: /* CMBLOC */
- NVME_GUEST_ERR(nvme_ub_mmiowr_cmbloc_reserved,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_cmbloc_reserved,
"invalid write to reserved CMBLOC"
" when CMBSZ is zero, ignored");
return;
case 0x3C: /* CMBSZ */
- NVME_GUEST_ERR(nvme_ub_mmiowr_cmbsz_readonly,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_cmbsz_readonly,
"invalid write to read only CMBSZ, ignored");
return;
default:
- NVME_GUEST_ERR(nvme_ub_mmiowr_invalid,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiowr_invalid,
"invalid MMIO write,"
" offset=0x%"PRIx64", data=%"PRIx64"",
offset, data);
@@ -1160,12 +1158,12 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
uint64_t val = 0;
if (unlikely(addr & (sizeof(uint32_t) - 1))) {
- NVME_GUEST_ERR(nvme_ub_mmiord_misaligned32,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiord_misaligned32,
"MMIO read not 32-bit aligned,"
" offset=0x%"PRIx64"", addr);
/* should RAZ, fall through for now */
} else if (unlikely(size < sizeof(uint32_t))) {
- NVME_GUEST_ERR(nvme_ub_mmiord_toosmall,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiord_toosmall,
"MMIO read smaller than 32-bits,"
" offset=0x%"PRIx64"", addr);
/* should RAZ, fall through for now */
@@ -1174,7 +1172,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
if (addr < sizeof(n->bar)) {
memcpy(&val, ptr + addr, size);
} else {
- NVME_GUEST_ERR(nvme_ub_mmiord_invalid_ofs,
+ NVME_GUEST_ERR(pci_nvme_ub_mmiord_invalid_ofs,
"MMIO read beyond last register,"
" offset=0x%"PRIx64", returning 0", addr);
}
@@ -1187,7 +1185,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
uint32_t qid;
if (unlikely(addr & ((1 << 2) - 1))) {
- NVME_GUEST_ERR(nvme_ub_db_wr_misaligned,
+ NVME_GUEST_ERR(pci_nvme_ub_db_wr_misaligned,
"doorbell write not 32-bit aligned,"
" offset=0x%"PRIx64", ignoring", addr);
return;
@@ -1202,7 +1200,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
qid = (addr - (0x1000 + (1 << 2))) >> 3;
if (unlikely(nvme_check_cqid(n, qid))) {
- NVME_GUEST_ERR(nvme_ub_db_wr_invalid_cq,
+ NVME_GUEST_ERR(pci_nvme_ub_db_wr_invalid_cq,
"completion queue doorbell write"
" for nonexistent queue,"
" sqid=%"PRIu32", ignoring", qid);
@@ -1211,7 +1209,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
cq = n->cq[qid];
if (unlikely(new_head >= cq->size)) {
- NVME_GUEST_ERR(nvme_ub_db_wr_invalid_cqhead,
+ NVME_GUEST_ERR(pci_nvme_ub_db_wr_invalid_cqhead,
"completion queue doorbell write value"
" beyond queue size, sqid=%"PRIu32","
" new_head=%"PRIu16", ignoring",
@@ -1240,7 +1238,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
qid = (addr - 0x1000) >> 3;
if (unlikely(nvme_check_sqid(n, qid))) {
- NVME_GUEST_ERR(nvme_ub_db_wr_invalid_sq,
+ NVME_GUEST_ERR(pci_nvme_ub_db_wr_invalid_sq,
"submission queue doorbell write"
" for nonexistent queue,"
" sqid=%"PRIu32", ignoring", qid);
@@ -1249,7 +1247,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
sq = n->sq[qid];
if (unlikely(new_tail >= sq->size)) {
- NVME_GUEST_ERR(nvme_ub_db_wr_invalid_sqtail,
+ NVME_GUEST_ERR(pci_nvme_ub_db_wr_invalid_sqtail,
"submission queue doorbell write value"
" beyond queue size, sqid=%"PRIu32","
" new_tail=%"PRIu16", ignoring",
diff --git a/hw/block/trace-events b/hw/block/trace-events
index bf6d11b58b85..1725abd92121 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -29,96 +29,96 @@ hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int t
# nvme.c
# nvme traces for successful events
-nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u"
-nvme_irq_pin(void) "pulsing IRQ pin"
-nvme_irq_masked(void) "IRQ is masked"
-nvme_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64" prp2=0x%"PRIx64""
-nvme_rw(const char *verb, uint32_t blk_count, uint64_t byte_count, uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64""
-nvme_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16""
-nvme_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", cqid=%"PRIu16", vector=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16", ien=%d"
-nvme_del_sq(uint16_t qid) "deleting submission queue sqid=%"PRIu16""
-nvme_del_cq(uint16_t cqid) "deleted completion queue, cqid=%"PRIu16""
-nvme_identify_ctrl(void) "identify controller"
-nvme_identify_ns(uint16_t ns) "identify namespace, nsid=%"PRIu16""
-nvme_identify_nslist(uint16_t ns) "identify namespace list, nsid=%"PRIu16""
-nvme_getfeat_vwcache(const char* result) "get feature volatile write cache, result=%s"
-nvme_getfeat_numq(int result) "get feature number of queues, result=%d"
-nvme_setfeat_numq(int reqcq, int reqsq, int gotcq, int gotsq) "requested cq_count=%d sq_count=%d, responding with cq_count=%d sq_count=%d"
-nvme_setfeat_timestamp(uint64_t ts) "set feature timestamp = 0x%"PRIx64""
-nvme_getfeat_timestamp(uint64_t ts) "get feature timestamp = 0x%"PRIx64""
-nvme_mmio_intm_set(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask set, data=0x%"PRIx64", new_mask=0x%"PRIx64""
-nvme_mmio_intm_clr(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask clr, data=0x%"PRIx64", new_mask=0x%"PRIx64""
-nvme_mmio_cfg(uint64_t data) "wrote MMIO, config controller config=0x%"PRIx64""
-nvme_mmio_aqattr(uint64_t data) "wrote MMIO, admin queue attributes=0x%"PRIx64""
-nvme_mmio_asqaddr(uint64_t data) "wrote MMIO, admin submission queue address=0x%"PRIx64""
-nvme_mmio_acqaddr(uint64_t data) "wrote MMIO, admin completion queue address=0x%"PRIx64""
-nvme_mmio_asqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin submission queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
-nvme_mmio_acqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin completion queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
-nvme_mmio_start_success(void) "setting controller enable bit succeeded"
-nvme_mmio_stopped(void) "cleared controller enable bit"
-nvme_mmio_shutdown_set(void) "shutdown bit set"
-nvme_mmio_shutdown_cleared(void) "shutdown bit cleared"
+pci_nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u"
+pci_nvme_irq_pin(void) "pulsing IRQ pin"
+pci_nvme_irq_masked(void) "IRQ is masked"
+pci_nvme_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64" prp2=0x%"PRIx64""
+pci_nvme_rw(const char *verb, uint32_t blk_count, uint64_t byte_count, uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64""
+pci_nvme_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16""
+pci_nvme_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", cqid=%"PRIu16", vector=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16", ien=%d"
+pci_nvme_del_sq(uint16_t qid) "deleting submission queue sqid=%"PRIu16""
+pci_nvme_del_cq(uint16_t cqid) "deleted completion queue, cqid=%"PRIu16""
+pci_nvme_identify_ctrl(void) "identify controller"
+pci_nvme_identify_ns(uint16_t ns) "identify namespace, nsid=%"PRIu16""
+pci_nvme_identify_nslist(uint16_t ns) "identify namespace list, nsid=%"PRIu16""
+pci_nvme_getfeat_vwcache(const char* result) "get feature volatile write cache, result=%s"
+pci_nvme_getfeat_numq(int result) "get feature number of queues, result=%d"
+pci_nvme_setfeat_numq(int reqcq, int reqsq, int gotcq, int gotsq) "requested cq_count=%d sq_count=%d, responding with cq_count=%d sq_count=%d"
+pci_nvme_setfeat_timestamp(uint64_t ts) "set feature timestamp = 0x%"PRIx64""
+pci_nvme_getfeat_timestamp(uint64_t ts) "get feature timestamp = 0x%"PRIx64""
+pci_nvme_mmio_intm_set(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask set, data=0x%"PRIx64", new_mask=0x%"PRIx64""
+pci_nvme_mmio_intm_clr(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask clr, data=0x%"PRIx64", new_mask=0x%"PRIx64""
+pci_nvme_mmio_cfg(uint64_t data) "wrote MMIO, config controller config=0x%"PRIx64""
+pci_nvme_mmio_aqattr(uint64_t data) "wrote MMIO, admin queue attributes=0x%"PRIx64""
+pci_nvme_mmio_asqaddr(uint64_t data) "wrote MMIO, admin submission queue address=0x%"PRIx64""
+pci_nvme_mmio_acqaddr(uint64_t data) "wrote MMIO, admin completion queue address=0x%"PRIx64""
+pci_nvme_mmio_asqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin submission queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
+pci_nvme_mmio_acqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin completion queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
+pci_nvme_mmio_start_success(void) "setting controller enable bit succeeded"
+pci_nvme_mmio_stopped(void) "cleared controller enable bit"
+pci_nvme_mmio_shutdown_set(void) "shutdown bit set"
+pci_nvme_mmio_shutdown_cleared(void) "shutdown bit cleared"
# nvme traces for error conditions
-nvme_err_invalid_dma(void) "PRP/SGL is too small for transfer size"
-nvme_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null or not page aligned: 0x%"PRIx64""
-nvme_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: 0x%"PRIx64""
-nvme_err_invalid_prp2_missing(void) "PRP2 is null and more data to be transferred"
-nvme_err_invalid_prp(void) "invalid PRP"
-nvme_err_invalid_ns(uint32_t ns, uint32_t limit) "invalid namespace %u not within 1-%u"
-nvme_err_invalid_opc(uint8_t opc) "invalid opcode 0x%"PRIx8""
-nvme_err_invalid_admin_opc(uint8_t opc) "invalid admin opcode 0x%"PRIx8""
-nvme_err_invalid_lba_range(uint64_t start, uint64_t len, uint64_t limit) "Invalid LBA start=%"PRIu64" len=%"PRIu64" limit=%"PRIu64""
-nvme_err_invalid_del_sq(uint16_t qid) "invalid submission queue deletion, sid=%"PRIu16""
-nvme_err_invalid_create_sq_cqid(uint16_t cqid) "failed creating submission queue, invalid cqid=%"PRIu16""
-nvme_err_invalid_create_sq_sqid(uint16_t sqid) "failed creating submission queue, invalid sqid=%"PRIu16""
-nvme_err_invalid_create_sq_size(uint16_t qsize) "failed creating submission queue, invalid qsize=%"PRIu16""
-nvme_err_invalid_create_sq_addr(uint64_t addr) "failed creating submission queue, addr=0x%"PRIx64""
-nvme_err_invalid_create_sq_qflags(uint16_t qflags) "failed creating submission queue, qflags=%"PRIu16""
-nvme_err_invalid_del_cq_cqid(uint16_t cqid) "failed deleting completion queue, cqid=%"PRIu16""
-nvme_err_invalid_del_cq_notempty(uint16_t cqid) "failed deleting completion queue, it is not empty, cqid=%"PRIu16""
-nvme_err_invalid_create_cq_cqid(uint16_t cqid) "failed creating completion queue, cqid=%"PRIu16""
-nvme_err_invalid_create_cq_size(uint16_t size) "failed creating completion queue, size=%"PRIu16""
-nvme_err_invalid_create_cq_addr(uint64_t addr) "failed creating completion queue, addr=0x%"PRIx64""
-nvme_err_invalid_create_cq_vector(uint16_t vector) "failed creating completion queue, vector=%"PRIu16""
-nvme_err_invalid_create_cq_qflags(uint16_t qflags) "failed creating completion queue, qflags=%"PRIu16""
-nvme_err_invalid_identify_cns(uint16_t cns) "identify, invalid cns=0x%"PRIx16""
-nvme_err_invalid_getfeat(int dw10) "invalid get features, dw10=0x%"PRIx32""
-nvme_err_invalid_setfeat(uint32_t dw10) "invalid set features, dw10=0x%"PRIx32""
-nvme_err_startfail_cq(void) "nvme_start_ctrl failed because there are non-admin completion queues"
-nvme_err_startfail_sq(void) "nvme_start_ctrl failed because there are non-admin submission queues"
-nvme_err_startfail_nbarasq(void) "nvme_start_ctrl failed because the admin submission queue address is null"
-nvme_err_startfail_nbaracq(void) "nvme_start_ctrl failed because the admin completion queue address is null"
-nvme_err_startfail_asq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin submission queue address is misaligned: 0x%"PRIx64""
-nvme_err_startfail_acq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin completion queue address is misaligned: 0x%"PRIx64""
-nvme_err_startfail_page_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too small: log2size=%u, min=%u"
-nvme_err_startfail_page_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too large: log2size=%u, max=%u"
-nvme_err_startfail_cqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too small: log2size=%u, min=%u"
-nvme_err_startfail_cqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too large: log2size=%u, max=%u"
-nvme_err_startfail_sqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too small: log2size=%u, min=%u"
-nvme_err_startfail_sqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too large: log2size=%u, max=%u"
-nvme_err_startfail_asqent_sz_zero(void) "nvme_start_ctrl failed because the admin submission queue size is zero"
-nvme_err_startfail_acqent_sz_zero(void) "nvme_start_ctrl failed because the admin completion queue size is zero"
-nvme_err_startfail(void) "setting controller enable bit failed"
+pci_nvme_err_invalid_dma(void) "PRP/SGL is too small for transfer size"
+pci_nvme_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null or not page aligned: 0x%"PRIx64""
+pci_nvme_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: 0x%"PRIx64""
+pci_nvme_err_invalid_prp2_missing(void) "PRP2 is null and more data to be transferred"
+pci_nvme_err_invalid_prp(void) "invalid PRP"
+pci_nvme_err_invalid_ns(uint32_t ns, uint32_t limit) "invalid namespace %u not within 1-%u"
+pci_nvme_err_invalid_opc(uint8_t opc) "invalid opcode 0x%"PRIx8""
+pci_nvme_err_invalid_admin_opc(uint8_t opc) "invalid admin opcode 0x%"PRIx8""
+pci_nvme_err_invalid_lba_range(uint64_t start, uint64_t len, uint64_t limit) "Invalid LBA start=%"PRIu64" len=%"PRIu64" limit=%"PRIu64""
+pci_nvme_err_invalid_del_sq(uint16_t qid) "invalid submission queue deletion, sid=%"PRIu16""
+pci_nvme_err_invalid_create_sq_cqid(uint16_t cqid) "failed creating submission queue, invalid cqid=%"PRIu16""
+pci_nvme_err_invalid_create_sq_sqid(uint16_t sqid) "failed creating submission queue, invalid sqid=%"PRIu16""
+pci_nvme_err_invalid_create_sq_size(uint16_t qsize) "failed creating submission queue, invalid qsize=%"PRIu16""
+pci_nvme_err_invalid_create_sq_addr(uint64_t addr) "failed creating submission queue, addr=0x%"PRIx64""
+pci_nvme_err_invalid_create_sq_qflags(uint16_t qflags) "failed creating submission queue, qflags=%"PRIu16""
+pci_nvme_err_invalid_del_cq_cqid(uint16_t cqid) "failed deleting completion queue, cqid=%"PRIu16""
+pci_nvme_err_invalid_del_cq_notempty(uint16_t cqid) "failed deleting completion queue, it is not empty, cqid=%"PRIu16""
+pci_nvme_err_invalid_create_cq_cqid(uint16_t cqid) "failed creating completion queue, cqid=%"PRIu16""
+pci_nvme_err_invalid_create_cq_size(uint16_t size) "failed creating completion queue, size=%"PRIu16""
+pci_nvme_err_invalid_create_cq_addr(uint64_t addr) "failed creating completion queue, addr=0x%"PRIx64""
+pci_nvme_err_invalid_create_cq_vector(uint16_t vector) "failed creating completion queue, vector=%"PRIu16""
+pci_nvme_err_invalid_create_cq_qflags(uint16_t qflags) "failed creating completion queue, qflags=%"PRIu16""
+pci_nvme_err_invalid_identify_cns(uint16_t cns) "identify, invalid cns=0x%"PRIx16""
+pci_nvme_err_invalid_getfeat(int dw10) "invalid get features, dw10=0x%"PRIx32""
+pci_nvme_err_invalid_setfeat(uint32_t dw10) "invalid set features, dw10=0x%"PRIx32""
+pci_nvme_err_startfail_cq(void) "nvme_start_ctrl failed because there are non-admin completion queues"
+pci_nvme_err_startfail_sq(void) "nvme_start_ctrl failed because there are non-admin submission queues"
+pci_nvme_err_startfail_nbarasq(void) "nvme_start_ctrl failed because the admin submission queue address is null"
+pci_nvme_err_startfail_nbaracq(void) "nvme_start_ctrl failed because the admin completion queue address is null"
+pci_nvme_err_startfail_asq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin submission queue address is misaligned: 0x%"PRIx64""
+pci_nvme_err_startfail_acq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin completion queue address is misaligned: 0x%"PRIx64""
+pci_nvme_err_startfail_page_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too small: log2size=%u, min=%u"
+pci_nvme_err_startfail_page_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too large: log2size=%u, max=%u"
+pci_nvme_err_startfail_cqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too small: log2size=%u, min=%u"
+pci_nvme_err_startfail_cqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too large: log2size=%u, max=%u"
+pci_nvme_err_startfail_sqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too small: log2size=%u, min=%u"
+pci_nvme_err_startfail_sqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too large: log2size=%u, max=%u"
+pci_nvme_err_startfail_asqent_sz_zero(void) "nvme_start_ctrl failed because the admin submission queue size is zero"
+pci_nvme_err_startfail_acqent_sz_zero(void) "nvme_start_ctrl failed because the admin completion queue size is zero"
+pci_nvme_err_startfail(void) "setting controller enable bit failed"
# Traces for undefined behavior
-nvme_ub_mmiowr_misaligned32(uint64_t offset) "MMIO write not 32-bit aligned, offset=0x%"PRIx64""
-nvme_ub_mmiowr_toosmall(uint64_t offset, unsigned size) "MMIO write smaller than 32 bits, offset=0x%"PRIx64", size=%u"
-nvme_ub_mmiowr_intmask_with_msix(void) "undefined access to interrupt mask set when MSI-X is enabled"
-nvme_ub_mmiowr_ro_csts(void) "attempted to set a read only bit of controller status"
-nvme_ub_mmiowr_ssreset_w1c_unsupported(void) "attempted to W1C CSTS.NSSRO but CAP.NSSRS is zero (not supported)"
-nvme_ub_mmiowr_ssreset_unsupported(void) "attempted NVM subsystem reset but CAP.NSSRS is zero (not supported)"
-nvme_ub_mmiowr_cmbloc_reserved(void) "invalid write to reserved CMBLOC when CMBSZ is zero, ignored"
-nvme_ub_mmiowr_cmbsz_readonly(void) "invalid write to read only CMBSZ, ignored"
-nvme_ub_mmiowr_invalid(uint64_t offset, uint64_t data) "invalid MMIO write, offset=0x%"PRIx64", data=0x%"PRIx64""
-nvme_ub_mmiord_misaligned32(uint64_t offset) "MMIO read not 32-bit aligned, offset=0x%"PRIx64""
-nvme_ub_mmiord_toosmall(uint64_t offset) "MMIO read smaller than 32-bits, offset=0x%"PRIx64""
-nvme_ub_mmiord_invalid_ofs(uint64_t offset) "MMIO read beyond last register, offset=0x%"PRIx64", returning 0"
-nvme_ub_db_wr_misaligned(uint64_t offset) "doorbell write not 32-bit aligned, offset=0x%"PRIx64", ignoring"
-nvme_ub_db_wr_invalid_cq(uint32_t qid) "completion queue doorbell write for nonexistent queue, cqid=%"PRIu32", ignoring"
-nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t new_head) "completion queue doorbell write value beyond queue size, cqid=%"PRIu32", new_head=%"PRIu16", ignoring"
-nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write for nonexistent queue, sqid=%"PRIu32", ignoring"
-nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail) "submission queue doorbell write value beyond queue size, sqid=%"PRIu32", new_head=%"PRIu16", ignoring"
+pci_nvme_ub_mmiowr_misaligned32(uint64_t offset) "MMIO write not 32-bit aligned, offset=0x%"PRIx64""
+pci_nvme_ub_mmiowr_toosmall(uint64_t offset, unsigned size) "MMIO write smaller than 32 bits, offset=0x%"PRIx64", size=%u"
+pci_nvme_ub_mmiowr_intmask_with_msix(void) "undefined access to interrupt mask set when MSI-X is enabled"
+pci_nvme_ub_mmiowr_ro_csts(void) "attempted to set a read only bit of controller status"
+pci_nvme_ub_mmiowr_ssreset_w1c_unsupported(void) "attempted to W1C CSTS.NSSRO but CAP.NSSRS is zero (not supported)"
+pci_nvme_ub_mmiowr_ssreset_unsupported(void) "attempted NVM subsystem reset but CAP.NSSRS is zero (not supported)"
+pci_nvme_ub_mmiowr_cmbloc_reserved(void) "invalid write to reserved CMBLOC when CMBSZ is zero, ignored"
+pci_nvme_ub_mmiowr_cmbsz_readonly(void) "invalid write to read only CMBSZ, ignored"
+pci_nvme_ub_mmiowr_invalid(uint64_t offset, uint64_t data) "invalid MMIO write, offset=0x%"PRIx64", data=0x%"PRIx64""
+pci_nvme_ub_mmiord_misaligned32(uint64_t offset) "MMIO read not 32-bit aligned, offset=0x%"PRIx64""
+pci_nvme_ub_mmiord_toosmall(uint64_t offset) "MMIO read smaller than 32-bits, offset=0x%"PRIx64""
+pci_nvme_ub_mmiord_invalid_ofs(uint64_t offset) "MMIO read beyond last register, offset=0x%"PRIx64", returning 0"
+pci_nvme_ub_db_wr_misaligned(uint64_t offset) "doorbell write not 32-bit aligned, offset=0x%"PRIx64", ignoring"
+pci_nvme_ub_db_wr_invalid_cq(uint32_t qid) "completion queue doorbell write for nonexistent queue, cqid=%"PRIu32", ignoring"
+pci_nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t new_head) "completion queue doorbell write value beyond queue size, cqid=%"PRIu32", new_head=%"PRIu16", ignoring"
+pci_nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write for nonexistent queue, sqid=%"PRIu32", ignoring"
+pci_nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail) "submission queue doorbell write value beyond queue size, sqid=%"PRIu32", new_head=%"PRIu16", ignoring"
# xen-block.c
xen_block_realize(const char *type, uint32_t disk, uint32_t partition) "%s d%up%u"
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 03/16] nvme: remove superfluous breaks
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 01/16] nvme: fix pci doorbell size calculation Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 02/16] nvme: rename trace events to pci_nvme Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 04/16] nvme: move device parameters to separate struct Klaus Jensen
` (16 subsequent siblings)
19 siblings, 0 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
These break statements was left over when commit 3036a626e9ef ("nvme:
add Get/Set Feature Timestamp support") was merged.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Acked-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 931ddeb26ba0..a947050209a1 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -791,7 +791,6 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
break;
case NVME_TIMESTAMP:
return nvme_get_feature_timestamp(n, cmd);
- break;
default:
trace_pci_nvme_err_invalid_getfeat(dw10);
return NVME_INVALID_FIELD | NVME_DNR;
@@ -835,11 +834,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
req->cqe.result =
cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
break;
-
case NVME_TIMESTAMP:
return nvme_set_feature_timestamp(n, cmd);
- break;
-
default:
trace_pci_nvme_err_invalid_setfeat(dw10);
return NVME_INVALID_FIELD | NVME_DNR;
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 04/16] nvme: move device parameters to separate struct
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (2 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 03/16] nvme: remove superfluous breaks Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 05/16] nvme: use constants in identify Klaus Jensen
` (15 subsequent siblings)
19 siblings, 0 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Move device configuration parameters to separate struct to make it
explicit what is configurable and what is set internally.
Signed-off-by: Klaus Jensen <klaus.jensen@cnexlabs.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Acked-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 45 +++++++++++++++++++++++----------------------
hw/block/nvme.h | 16 +++++++++++++---
2 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index a947050209a1..a2b15a0ebd87 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -67,12 +67,12 @@ static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
{
- return sqid < n->num_queues && n->sq[sqid] != NULL ? 0 : -1;
+ return sqid < n->params.num_queues && n->sq[sqid] != NULL ? 0 : -1;
}
static int nvme_check_cqid(NvmeCtrl *n, uint16_t cqid)
{
- return cqid < n->num_queues && n->cq[cqid] != NULL ? 0 : -1;
+ return cqid < n->params.num_queues && n->cq[cqid] != NULL ? 0 : -1;
}
static void nvme_inc_cq_tail(NvmeCQueue *cq)
@@ -634,7 +634,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
trace_pci_nvme_err_invalid_create_cq_addr(prp1);
return NVME_INVALID_FIELD | NVME_DNR;
}
- if (unlikely(vector > n->num_queues)) {
+ if (unlikely(vector > n->params.num_queues)) {
trace_pci_nvme_err_invalid_create_cq_vector(vector);
return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
}
@@ -786,7 +786,8 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
break;
case NVME_NUMBER_OF_QUEUES:
- result = cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
+ result = cpu_to_le32((n->params.num_queues - 2) |
+ ((n->params.num_queues - 2) << 16));
trace_pci_nvme_getfeat_numq(result);
break;
case NVME_TIMESTAMP:
@@ -830,9 +831,10 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
case NVME_NUMBER_OF_QUEUES:
trace_pci_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
((dw11 >> 16) & 0xFFFF) + 1,
- n->num_queues - 1, n->num_queues - 1);
- req->cqe.result =
- cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
+ n->params.num_queues - 1,
+ n->params.num_queues - 1);
+ req->cqe.result = cpu_to_le32((n->params.num_queues - 2) |
+ ((n->params.num_queues - 2) << 16));
break;
case NVME_TIMESTAMP:
return nvme_set_feature_timestamp(n, cmd);
@@ -903,12 +905,12 @@ static void nvme_clear_ctrl(NvmeCtrl *n)
blk_drain(n->conf.blk);
- for (i = 0; i < n->num_queues; i++) {
+ for (i = 0; i < n->params.num_queues; i++) {
if (n->sq[i] != NULL) {
nvme_free_sq(n->sq[i], n);
}
}
- for (i = 0; i < n->num_queues; i++) {
+ for (i = 0; i < n->params.num_queues; i++) {
if (n->cq[i] != NULL) {
nvme_free_cq(n->cq[i], n);
}
@@ -1309,7 +1311,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
int64_t bs_size;
uint8_t *pci_conf;
- if (!n->num_queues) {
+ if (!n->params.num_queues) {
error_setg(errp, "num_queues can't be zero");
return;
}
@@ -1325,7 +1327,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
- if (!n->serial) {
+ if (!n->params.serial) {
error_setg(errp, "serial property not set");
return;
}
@@ -1344,25 +1346,26 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
n->num_namespaces = 1;
/* num_queues is really number of pairs, so each has two doorbells */
- n->reg_size = pow2ceil(NVME_REG_SIZE + 2 * n->num_queues * NVME_DB_SIZE);
+ n->reg_size = pow2ceil(NVME_REG_SIZE +
+ 2 * n->params.num_queues * NVME_DB_SIZE);
n->ns_size = bs_size / (uint64_t)n->num_namespaces;
n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
- n->sq = g_new0(NvmeSQueue *, n->num_queues);
- n->cq = g_new0(NvmeCQueue *, n->num_queues);
+ n->sq = g_new0(NvmeSQueue *, n->params.num_queues);
+ n->cq = g_new0(NvmeCQueue *, n->params.num_queues);
memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
"nvme", n->reg_size);
pci_register_bar(pci_dev, 0,
PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
&n->iomem);
- msix_init_exclusive_bar(pci_dev, n->num_queues, 4, NULL);
+ msix_init_exclusive_bar(pci_dev, n->params.num_queues, 4, NULL);
id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
strpadcpy((char *)id->fr, sizeof(id->fr), "1.0", ' ');
- strpadcpy((char *)id->sn, sizeof(id->sn), n->serial, ' ');
+ strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
id->rab = 6;
id->ieee[0] = 0x00;
id->ieee[1] = 0x02;
@@ -1391,7 +1394,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
n->bar.vs = 0x00010200;
n->bar.intmc = n->bar.intms = 0;
- if (n->cmb_size_mb) {
+ if (n->params.cmb_size_mb) {
NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2);
NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
@@ -1402,7 +1405,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
- NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->cmb_size_mb);
+ NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
n->cmbloc = n->bar.cmbloc;
n->cmbsz = n->bar.cmbsz;
@@ -1441,7 +1444,7 @@ static void nvme_exit(PCIDevice *pci_dev)
g_free(n->cq);
g_free(n->sq);
- if (n->cmb_size_mb) {
+ if (n->params.cmb_size_mb) {
g_free(n->cmbuf);
}
msix_uninit_exclusive_bar(pci_dev);
@@ -1449,9 +1452,7 @@ static void nvme_exit(PCIDevice *pci_dev)
static Property nvme_props[] = {
DEFINE_BLOCK_PROPERTIES(NvmeCtrl, conf),
- DEFINE_PROP_STRING("serial", NvmeCtrl, serial),
- DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, cmb_size_mb, 0),
- DEFINE_PROP_UINT32("num_queues", NvmeCtrl, num_queues, 64),
+ DEFINE_NVME_PROPERTIES(NvmeCtrl, params),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 557194ee1954..9957c4a200e2 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -1,7 +1,19 @@
#ifndef HW_NVME_H
#define HW_NVME_H
+
#include "block/nvme.h"
+#define DEFINE_NVME_PROPERTIES(_state, _props) \
+ DEFINE_PROP_STRING("serial", _state, _props.serial), \
+ DEFINE_PROP_UINT32("cmb_size_mb", _state, _props.cmb_size_mb, 0), \
+ DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64)
+
+typedef struct NvmeParams {
+ char *serial;
+ uint32_t num_queues;
+ uint32_t cmb_size_mb;
+} NvmeParams;
+
typedef struct NvmeAsyncEvent {
QSIMPLEQ_ENTRY(NvmeAsyncEvent) entry;
NvmeAerResult result;
@@ -63,6 +75,7 @@ typedef struct NvmeCtrl {
MemoryRegion ctrl_mem;
NvmeBar bar;
BlockConf conf;
+ NvmeParams params;
uint32_t page_size;
uint16_t page_bits;
@@ -71,10 +84,8 @@ typedef struct NvmeCtrl {
uint16_t sqe_size;
uint32_t reg_size;
uint32_t num_namespaces;
- uint32_t num_queues;
uint32_t max_q_ents;
uint64_t ns_size;
- uint32_t cmb_size_mb;
uint32_t cmbsz;
uint32_t cmbloc;
uint8_t *cmbuf;
@@ -82,7 +93,6 @@ typedef struct NvmeCtrl {
uint64_t host_timestamp; /* Timestamp sent by the host */
uint64_t timestamp_set_qemu_clock_ms; /* QEMU clock time */
- char *serial;
NvmeNamespace *namespaces;
NvmeSQueue **sq;
NvmeCQueue **cq;
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 05/16] nvme: use constants in identify
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (3 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 04/16] nvme: move device parameters to separate struct Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 06/16] nvme: refactor nvme_addr_read Klaus Jensen
` (14 subsequent siblings)
19 siblings, 0 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 8 ++++----
include/block/nvme.h | 8 ++++++++
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index a2b15a0ebd87..563cccae6795 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -682,7 +682,7 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeIdentify *c)
static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeIdentify *c)
{
- static const int data_len = 4 * KiB;
+ static const int data_len = NVME_IDENTIFY_DATA_SIZE;
uint32_t min_nsid = le32_to_cpu(c->nsid);
uint64_t prp1 = le64_to_cpu(c->prp1);
uint64_t prp2 = le64_to_cpu(c->prp2);
@@ -712,11 +712,11 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
NvmeIdentify *c = (NvmeIdentify *)cmd;
switch (le32_to_cpu(c->cns)) {
- case 0x00:
+ case NVME_ID_CNS_NS:
return nvme_identify_ns(n, c);
- case 0x01:
+ case NVME_ID_CNS_CTRL:
return nvme_identify_ctrl(n, c);
- case 0x02:
+ case NVME_ID_CNS_NS_ACTIVE_LIST:
return nvme_identify_nslist(n, c);
default:
trace_pci_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 8fb941c6537c..c2fd01cf2f1d 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -533,6 +533,14 @@ typedef struct NvmePSD {
uint8_t resv[16];
} NvmePSD;
+#define NVME_IDENTIFY_DATA_SIZE 4096
+
+enum {
+ NVME_ID_CNS_NS = 0x0,
+ NVME_ID_CNS_CTRL = 0x1,
+ NVME_ID_CNS_NS_ACTIVE_LIST = 0x2,
+};
+
typedef struct NvmeIdCtrl {
uint16_t vid;
uint16_t ssvid;
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 06/16] nvme: refactor nvme_addr_read
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (4 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 05/16] nvme: use constants in identify Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-21 10:35 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 07/16] nvme: add max_ioqpairs device parameter Klaus Jensen
` (13 subsequent siblings)
19 siblings, 1 reply; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Pull the controller memory buffer check to its own function. The check
will be used on its own in later patches.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Acked-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 563cccae6795..d026985f62d0 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -55,14 +55,22 @@
static void nvme_process_sq(void *opaque);
+static bool nvme_addr_is_cmb(NvmeCtrl *n, hwaddr addr)
+{
+ hwaddr low = n->ctrl_mem.addr;
+ hwaddr hi = n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size);
+
+ return addr >= low && addr < hi;
+}
+
static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
{
- if (n->cmbsz && addr >= n->ctrl_mem.addr &&
- addr < (n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size))) {
+ if (n->cmbsz && nvme_addr_is_cmb(n, addr)) {
memcpy(buf, (void *)&n->cmbuf[addr - n->ctrl_mem.addr], size);
- } else {
- pci_dma_read(&n->parent_obj, addr, buf, size);
+ return;
}
+
+ pci_dma_read(&n->parent_obj, addr, buf, size);
}
static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 07/16] nvme: add max_ioqpairs device parameter
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (5 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 06/16] nvme: refactor nvme_addr_read Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 08/16] nvme: remove redundant cmbloc/cmbsz members Klaus Jensen
` (12 subsequent siblings)
19 siblings, 0 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
The num_queues device paramater has a slightly confusing meaning because
it accounts for the admin queue pair which is not really optional.
Secondly, it is really a maximum value of queues allowed.
Add a new max_ioqpairs parameter that only accounts for I/O queue pairs,
but keep num_queues for compatibility.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 48 +++++++++++++++++++++++++++++-------------------
hw/block/nvme.h | 6 ++++--
2 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d026985f62d0..8092c1b46eb1 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -19,7 +19,7 @@
* -drive file=<file>,if=none,id=<drive_id>
* -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \
* cmb_size_mb=<cmb_size_mb[optional]>, \
- * num_queues=<N[optional]>
+ * max_ioqpairs=<N[optional]>
*
* Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
* offset 0 in BAR2 and supports only WDS, RDS and SQS for now.
@@ -27,6 +27,7 @@
#include "qemu/osdep.h"
#include "qemu/units.h"
+#include "qemu/error-report.h"
#include "hw/block/block.h"
#include "hw/pci/msix.h"
#include "hw/pci/pci.h"
@@ -75,12 +76,12 @@ static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
{
- return sqid < n->params.num_queues && n->sq[sqid] != NULL ? 0 : -1;
+ return sqid < n->params.max_ioqpairs + 1 && n->sq[sqid] != NULL ? 0 : -1;
}
static int nvme_check_cqid(NvmeCtrl *n, uint16_t cqid)
{
- return cqid < n->params.num_queues && n->cq[cqid] != NULL ? 0 : -1;
+ return cqid < n->params.max_ioqpairs + 1 && n->cq[cqid] != NULL ? 0 : -1;
}
static void nvme_inc_cq_tail(NvmeCQueue *cq)
@@ -642,7 +643,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
trace_pci_nvme_err_invalid_create_cq_addr(prp1);
return NVME_INVALID_FIELD | NVME_DNR;
}
- if (unlikely(vector > n->params.num_queues)) {
+ if (unlikely(vector > n->params.max_ioqpairs + 1)) {
trace_pci_nvme_err_invalid_create_cq_vector(vector);
return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
}
@@ -794,8 +795,8 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
break;
case NVME_NUMBER_OF_QUEUES:
- result = cpu_to_le32((n->params.num_queues - 2) |
- ((n->params.num_queues - 2) << 16));
+ result = cpu_to_le32((n->params.max_ioqpairs - 1) |
+ ((n->params.max_ioqpairs - 1) << 16));
trace_pci_nvme_getfeat_numq(result);
break;
case NVME_TIMESTAMP:
@@ -839,10 +840,10 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
case NVME_NUMBER_OF_QUEUES:
trace_pci_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
((dw11 >> 16) & 0xFFFF) + 1,
- n->params.num_queues - 1,
- n->params.num_queues - 1);
- req->cqe.result = cpu_to_le32((n->params.num_queues - 2) |
- ((n->params.num_queues - 2) << 16));
+ n->params.max_ioqpairs,
+ n->params.max_ioqpairs);
+ req->cqe.result = cpu_to_le32((n->params.max_ioqpairs - 1) |
+ ((n->params.max_ioqpairs - 1) << 16));
break;
case NVME_TIMESTAMP:
return nvme_set_feature_timestamp(n, cmd);
@@ -913,12 +914,12 @@ static void nvme_clear_ctrl(NvmeCtrl *n)
blk_drain(n->conf.blk);
- for (i = 0; i < n->params.num_queues; i++) {
+ for (i = 0; i < n->params.max_ioqpairs + 1; i++) {
if (n->sq[i] != NULL) {
nvme_free_sq(n->sq[i], n);
}
}
- for (i = 0; i < n->params.num_queues; i++) {
+ for (i = 0; i < n->params.max_ioqpairs + 1; i++) {
if (n->cq[i] != NULL) {
nvme_free_cq(n->cq[i], n);
}
@@ -1319,8 +1320,17 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
int64_t bs_size;
uint8_t *pci_conf;
- if (!n->params.num_queues) {
- error_setg(errp, "num_queues can't be zero");
+ if (n->params.num_queues) {
+ warn_report("num_queues is deprecated; please use max_ioqpairs "
+ "instead");
+
+ n->params.max_ioqpairs = n->params.num_queues - 1;
+ }
+
+ if (n->params.max_ioqpairs < 1 ||
+ n->params.max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
+ error_setg(errp, "max_ioqpairs must be between 1 and %d",
+ PCI_MSIX_FLAGS_QSIZE);
return;
}
@@ -1353,21 +1363,21 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
n->num_namespaces = 1;
- /* num_queues is really number of pairs, so each has two doorbells */
+ /* add one to max_ioqpairs to account for the admin queue pair */
n->reg_size = pow2ceil(NVME_REG_SIZE +
- 2 * n->params.num_queues * NVME_DB_SIZE);
+ 2 * (n->params.max_ioqpairs + 1) * NVME_DB_SIZE);
n->ns_size = bs_size / (uint64_t)n->num_namespaces;
n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
- n->sq = g_new0(NvmeSQueue *, n->params.num_queues);
- n->cq = g_new0(NvmeCQueue *, n->params.num_queues);
+ n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
+ n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
"nvme", n->reg_size);
pci_register_bar(pci_dev, 0,
PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
&n->iomem);
- msix_init_exclusive_bar(pci_dev, n->params.num_queues, 4, NULL);
+ msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 9957c4a200e2..1617787af2e9 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -6,11 +6,13 @@
#define DEFINE_NVME_PROPERTIES(_state, _props) \
DEFINE_PROP_STRING("serial", _state, _props.serial), \
DEFINE_PROP_UINT32("cmb_size_mb", _state, _props.cmb_size_mb, 0), \
- DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64)
+ DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 0), \
+ DEFINE_PROP_UINT32("max_ioqpairs", _state, _props.max_ioqpairs, 64)
typedef struct NvmeParams {
char *serial;
- uint32_t num_queues;
+ uint32_t num_queues; /* deprecated since 5.1 */
+ uint32_t max_ioqpairs;
uint32_t cmb_size_mb;
} NvmeParams;
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 08/16] nvme: remove redundant cmbloc/cmbsz members
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (6 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 07/16] nvme: add max_ioqpairs device parameter Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-21 12:05 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 09/16] nvme: factor out property/constraint checks Klaus Jensen
` (11 subsequent siblings)
19 siblings, 1 reply; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 7 ++-----
hw/block/nvme.h | 2 --
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 8092c1b46eb1..44856e873fd1 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -66,7 +66,7 @@ static bool nvme_addr_is_cmb(NvmeCtrl *n, hwaddr addr)
static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
{
- if (n->cmbsz && nvme_addr_is_cmb(n, addr)) {
+ if (n->bar.cmbsz && nvme_addr_is_cmb(n, addr)) {
memcpy(buf, (void *)&n->cmbuf[addr - n->ctrl_mem.addr], size);
return;
}
@@ -160,7 +160,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
if (unlikely(!prp1)) {
trace_pci_nvme_err_invalid_prp();
return NVME_INVALID_FIELD | NVME_DNR;
- } else if (n->cmbsz && prp1 >= n->ctrl_mem.addr &&
+ } else if (n->bar.cmbsz && prp1 >= n->ctrl_mem.addr &&
prp1 < n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size)) {
qsg->nsg = 0;
qemu_iovec_init(iov, num_prps);
@@ -1425,9 +1425,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
- n->cmbloc = n->bar.cmbloc;
- n->cmbsz = n->bar.cmbsz;
-
n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
"nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 1617787af2e9..7eecfd3a50f6 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -88,8 +88,6 @@ typedef struct NvmeCtrl {
uint32_t num_namespaces;
uint32_t max_q_ents;
uint64_t ns_size;
- uint32_t cmbsz;
- uint32_t cmbloc;
uint8_t *cmbuf;
uint64_t irq_status;
uint64_t host_timestamp; /* Timestamp sent by the host */
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 09/16] nvme: factor out property/constraint checks
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (7 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 08/16] nvme: remove redundant cmbloc/cmbsz members Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:08 ` Philippe Mathieu-Daudé
2020-04-21 14:53 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 10/16] nvme: factor out device state setup Klaus Jensen
` (10 subsequent siblings)
19 siblings, 2 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 43 ++++++++++++++++++++++++++++---------------
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 44856e873fd1..5f9ebbd6a1d5 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1311,24 +1311,19 @@ static const MemoryRegionOps nvme_cmb_ops = {
},
};
-static void nvme_realize(PCIDevice *pci_dev, Error **errp)
+static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
{
- NvmeCtrl *n = NVME(pci_dev);
- NvmeIdCtrl *id = &n->id_ctrl;
+ NvmeParams *params = &n->params;
- int i;
- int64_t bs_size;
- uint8_t *pci_conf;
-
- if (n->params.num_queues) {
+ if (params->num_queues) {
warn_report("num_queues is deprecated; please use max_ioqpairs "
"instead");
- n->params.max_ioqpairs = n->params.num_queues - 1;
+ params->max_ioqpairs = params->num_queues - 1;
}
- if (n->params.max_ioqpairs < 1 ||
- n->params.max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
+ if (params->max_ioqpairs < 1 ||
+ params->max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
error_setg(errp, "max_ioqpairs must be between 1 and %d",
PCI_MSIX_FLAGS_QSIZE);
return;
@@ -1339,16 +1334,34 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
+ if (!params->serial) {
+ error_setg(errp, "serial property not set");
+ return;
+ }
+}
+
+static void nvme_realize(PCIDevice *pci_dev, Error **errp)
+{
+ NvmeCtrl *n = NVME(pci_dev);
+ NvmeIdCtrl *id = &n->id_ctrl;
+ Error *err = NULL;
+
+ int i;
+ int64_t bs_size;
+ uint8_t *pci_conf;
+
+ nvme_check_constraints(n, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
bs_size = blk_getlength(n->conf.blk);
if (bs_size < 0) {
error_setg(errp, "could not get backing file size");
return;
}
- if (!n->params.serial) {
- error_setg(errp, "serial property not set");
- return;
- }
blkconf_blocksizes(&n->conf);
if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
false, errp)) {
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 10/16] nvme: factor out device state setup
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (8 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 09/16] nvme: factor out property/constraint checks Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-21 14:55 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 11/16] nvme: factor out block backend setup Klaus Jensen
` (9 subsequent siblings)
19 siblings, 1 reply; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5f9ebbd6a1d5..45a352b63d89 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1340,6 +1340,17 @@ static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
}
}
+static void nvme_init_state(NvmeCtrl *n)
+{
+ n->num_namespaces = 1;
+ /* add one to max_ioqpairs to account for the admin queue pair */
+ n->reg_size = pow2ceil(NVME_REG_SIZE +
+ 2 * (n->params.max_ioqpairs + 1) * NVME_DB_SIZE);
+ n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
+ n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
+ n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
+}
+
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
@@ -1356,6 +1367,8 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
+ nvme_init_state(n);
+
bs_size = blk_getlength(n->conf.blk);
if (bs_size < 0) {
error_setg(errp, "could not get backing file size");
@@ -1374,17 +1387,8 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
pcie_endpoint_cap_init(pci_dev, 0x80);
- n->num_namespaces = 1;
-
- /* add one to max_ioqpairs to account for the admin queue pair */
- n->reg_size = pow2ceil(NVME_REG_SIZE +
- 2 * (n->params.max_ioqpairs + 1) * NVME_DB_SIZE);
n->ns_size = bs_size / (uint64_t)n->num_namespaces;
- n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
- n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
- n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
-
memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
"nvme", n->reg_size);
pci_register_bar(pci_dev, 0,
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 11/16] nvme: factor out block backend setup
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (9 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 10/16] nvme: factor out device state setup Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:08 ` Philippe Mathieu-Daudé
2020-04-21 15:04 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 12/16] nvme: add namespace helpers Klaus Jensen
` (8 subsequent siblings)
19 siblings, 2 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 45a352b63d89..80da0825d295 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1351,6 +1351,13 @@ static void nvme_init_state(NvmeCtrl *n)
n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
}
+static void nvme_init_blk(NvmeCtrl *n, Error **errp)
+{
+ blkconf_blocksizes(&n->conf);
+ blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
+ false, errp);
+}
+
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
@@ -1375,9 +1382,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
- blkconf_blocksizes(&n->conf);
- if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
- false, errp)) {
+ nvme_init_blk(n, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
}
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 12/16] nvme: add namespace helpers
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (10 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 11/16] nvme: factor out block backend setup Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:09 ` Philippe Mathieu-Daudé
2020-04-21 15:41 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 13/16] nvme: factor out namespace setup Klaus Jensen
` (7 subsequent siblings)
19 siblings, 2 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Introduce some small helpers to make the next patches easier on the eye.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 3 +--
hw/block/nvme.h | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 80da0825d295..d5244102252c 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1469,8 +1469,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
id_ns->dps = 0;
id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
id_ns->ncap = id_ns->nuse = id_ns->nsze =
- cpu_to_le64(n->ns_size >>
- id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds);
+ cpu_to_le64(nvme_ns_nlbas(n, ns));
}
}
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 7eecfd3a50f6..dd932a9e7ebc 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -67,6 +67,17 @@ typedef struct NvmeNamespace {
NvmeIdNs id_ns;
} NvmeNamespace;
+static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
+{
+ NvmeIdNs *id_ns = &ns->id_ns;
+ return &id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(id_ns->flbas)];
+}
+
+static inline uint8_t nvme_ns_lbads(NvmeNamespace *ns)
+{
+ return nvme_ns_lbaf(ns)->ds;
+}
+
#define TYPE_NVME "nvme"
#define NVME(obj) \
OBJECT_CHECK(NvmeCtrl, (obj), TYPE_NVME)
@@ -101,4 +112,9 @@ typedef struct NvmeCtrl {
NvmeIdCtrl id_ctrl;
} NvmeCtrl;
+static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
+{
+ return n->ns_size >> nvme_ns_lbads(ns);
+}
+
#endif /* HW_NVME_H */
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 13/16] nvme: factor out namespace setup
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (11 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 12/16] nvme: add namespace helpers Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:16 ` Philippe Mathieu-Daudé
2020-04-21 15:57 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 14/16] nvme: factor out pci setup Klaus Jensen
` (6 subsequent siblings)
19 siblings, 2 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 46 ++++++++++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d5244102252c..2b007115c302 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1358,6 +1358,27 @@ static void nvme_init_blk(NvmeCtrl *n, Error **errp)
false, errp);
}
+static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
+{
+ int64_t bs_size;
+ NvmeIdNs *id_ns = &ns->id_ns;
+
+ bs_size = blk_getlength(n->conf.blk);
+ if (bs_size < 0) {
+ error_setg_errno(errp, -bs_size, "could not get backing file size");
+ return;
+ }
+
+ n->ns_size = bs_size;
+
+ id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
+ id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(n, ns));
+
+ /* no thin provisioning */
+ id_ns->ncap = id_ns->nsze;
+ id_ns->nuse = id_ns->ncap;
+}
+
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
@@ -1365,7 +1386,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
Error *err = NULL;
int i;
- int64_t bs_size;
uint8_t *pci_conf;
nvme_check_constraints(n, &err);
@@ -1376,12 +1396,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
nvme_init_state(n);
- bs_size = blk_getlength(n->conf.blk);
- if (bs_size < 0) {
- error_setg(errp, "could not get backing file size");
- return;
- }
-
nvme_init_blk(n, &err);
if (err) {
error_propagate(errp, err);
@@ -1394,8 +1408,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
pcie_endpoint_cap_init(pci_dev, 0x80);
- n->ns_size = bs_size / (uint64_t)n->num_namespaces;
-
memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
"nvme", n->reg_size);
pci_register_bar(pci_dev, 0,
@@ -1459,17 +1471,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
}
for (i = 0; i < n->num_namespaces; i++) {
- NvmeNamespace *ns = &n->namespaces[i];
- NvmeIdNs *id_ns = &ns->id_ns;
- id_ns->nsfeat = 0;
- id_ns->nlbaf = 0;
- id_ns->flbas = 0;
- id_ns->mc = 0;
- id_ns->dpc = 0;
- id_ns->dps = 0;
- id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
- id_ns->ncap = id_ns->nuse = id_ns->nsze =
- cpu_to_le64(nvme_ns_nlbas(n, ns));
+ nvme_init_namespace(n, &n->namespaces[i], &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
}
}
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 14/16] nvme: factor out pci setup
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (12 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 13/16] nvme: factor out namespace setup Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:14 ` Philippe Mathieu-Daudé
2020-04-21 15:59 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 15/16] nvme: factor out cmb setup Klaus Jensen
` (5 subsequent siblings)
19 siblings, 2 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 2b007115c302..906ae595025a 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1379,6 +1379,22 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
id_ns->nuse = id_ns->ncap;
}
+static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
+{
+ uint8_t *pci_conf = pci_dev->config;
+
+ pci_conf[PCI_INTERRUPT_PIN] = 1;
+ pci_config_set_prog_interface(pci_conf, 0x2);
+ pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
+ pcie_endpoint_cap_init(pci_dev, 0x80);
+
+ memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n, "nvme",
+ n->reg_size);
+ pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem);
+ msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
+}
+
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
@@ -1402,19 +1418,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
+ nvme_init_pci(n, pci_dev);
+
pci_conf = pci_dev->config;
- pci_conf[PCI_INTERRUPT_PIN] = 1;
- pci_config_set_prog_interface(pci_dev->config, 0x2);
- pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
- pcie_endpoint_cap_init(pci_dev, 0x80);
-
- memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
- "nvme", n->reg_size);
- pci_register_bar(pci_dev, 0,
- PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
- &n->iomem);
- msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
-
id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 15/16] nvme: factor out cmb setup
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (13 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 14/16] nvme: factor out pci setup Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-21 16:10 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 16/16] nvme: factor out controller identify setup Klaus Jensen
` (4 subsequent siblings)
19 siblings, 1 reply; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 49 +++++++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 906ae595025a..4c28d75e0fc8 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -46,6 +46,7 @@
#define NVME_REG_SIZE 0x1000
#define NVME_DB_SIZE 4
+#define NVME_CMB_BIR 2
#define NVME_GUEST_ERR(trace, fmt, ...) \
do { \
@@ -1379,6 +1380,28 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
id_ns->nuse = id_ns->ncap;
}
+static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
+{
+ NVME_CMBLOC_SET_BIR(n->bar.cmbloc, NVME_CMB_BIR);
+ NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
+
+ NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
+ NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
+ NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
+ NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
+ NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
+ NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
+ NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
+
+ n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
+ memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
+ "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
+ pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_TYPE_64 |
+ PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
+}
+
static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
{
uint8_t *pci_conf = pci_dev->config;
@@ -1393,6 +1416,10 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem);
msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
+
+ if (n->params.cmb_size_mb) {
+ nvme_init_cmb(n, pci_dev);
+ }
}
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
@@ -1454,28 +1481,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
n->bar.vs = 0x00010200;
n->bar.intmc = n->bar.intms = 0;
- if (n->params.cmb_size_mb) {
-
- NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2);
- NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
-
- NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
- NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
- NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
- NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
- NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
- NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
- NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
-
- n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
- memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
- "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
- pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
- PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |
- PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
-
- }
-
for (i = 0; i < n->num_namespaces; i++) {
nvme_init_namespace(n, &n->namespaces[i], &err);
if (err) {
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 16/16] nvme: factor out controller identify setup
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (14 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 15/16] nvme: factor out cmb setup Klaus Jensen
@ 2020-04-15 13:01 ` Klaus Jensen
2020-04-15 13:06 ` Philippe Mathieu-Daudé
2020-04-21 16:17 ` Maxim Levitsky
2020-04-15 14:29 ` [PATCH v2 00/16] nvme: refactoring and cleanups no-reply
` (3 subsequent siblings)
19 siblings, 2 replies; 48+ messages in thread
From: Klaus Jensen @ 2020-04-15 13:01 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Klaus Jensen, Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 52 +++++++++++++++++++++++++++----------------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 4c28d75e0fc8..804f24719dce 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1422,32 +1422,11 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
}
}
-static void nvme_realize(PCIDevice *pci_dev, Error **errp)
+static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
{
- NvmeCtrl *n = NVME(pci_dev);
NvmeIdCtrl *id = &n->id_ctrl;
- Error *err = NULL;
+ uint8_t *pci_conf = pci_dev->config;
- int i;
- uint8_t *pci_conf;
-
- nvme_check_constraints(n, &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
-
- nvme_init_state(n);
-
- nvme_init_blk(n, &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
-
- nvme_init_pci(n, pci_dev);
-
- pci_conf = pci_dev->config;
id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
@@ -1481,6 +1460,33 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
n->bar.vs = 0x00010200;
n->bar.intmc = n->bar.intms = 0;
+
+}
+
+static void nvme_realize(PCIDevice *pci_dev, Error **errp)
+{
+ NvmeCtrl *n = NVME(pci_dev);
+ Error *err = NULL;
+
+ int i;
+
+ nvme_check_constraints(n, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+
+ nvme_init_state(n);
+ nvme_init_blk(n, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ nvme_init_pci(n, pci_dev);
+ nvme_init_ctrl(n, pci_dev);
+
for (i = 0; i < n->num_namespaces; i++) {
nvme_init_namespace(n, &n->namespaces[i], &err);
if (err) {
--
2.26.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 02/16] nvme: rename trace events to pci_nvme
2020-04-15 13:01 ` [PATCH v2 02/16] nvme: rename trace events to pci_nvme Klaus Jensen
@ 2020-04-15 13:04 ` Philippe Mathieu-Daudé
2020-04-21 9:43 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:04 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
On 4/15/20 3:01 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Change the prefix of all nvme device related trace events to 'pci_nvme'
> to not clash with trace events from the nvme block driver.
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 190 +++++++++++++++++++++---------------------
> hw/block/trace-events | 172 +++++++++++++++++++-------------------
> 2 files changed, 180 insertions(+), 182 deletions(-)
Thanks for updating.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 16/16] nvme: factor out controller identify setup
2020-04-15 13:01 ` [PATCH v2 16/16] nvme: factor out controller identify setup Klaus Jensen
@ 2020-04-15 13:06 ` Philippe Mathieu-Daudé
2020-04-21 16:17 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:06 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
On 4/15/20 3:01 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/nvme.c | 52 +++++++++++++++++++++++++++----------------------
> 1 file changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 4c28d75e0fc8..804f24719dce 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1422,32 +1422,11 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> }
> }
>
> -static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> +static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> {
> - NvmeCtrl *n = NVME(pci_dev);
> NvmeIdCtrl *id = &n->id_ctrl;
> - Error *err = NULL;
> + uint8_t *pci_conf = pci_dev->config;
>
> - int i;
> - uint8_t *pci_conf;
> -
> - nvme_check_constraints(n, &err);
> - if (err) {
> - error_propagate(errp, err);
> - return;
> - }
> -
> - nvme_init_state(n);
> -
> - nvme_init_blk(n, &err);
> - if (err) {
> - error_propagate(errp, err);
> - return;
> - }
> -
> - nvme_init_pci(n, pci_dev);
> -
> - pci_conf = pci_dev->config;
> id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
> id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
> strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
> @@ -1481,6 +1460,33 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> n->bar.vs = 0x00010200;
> n->bar.intmc = n->bar.intms = 0;
>
> +
Spurious empty lines (no need to repost!).
To other reviewer, patch trivial to review with 'git-diff -W'.
> +}
> +
> +static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> +{
> + NvmeCtrl *n = NVME(pci_dev);
> + Error *err = NULL;
> +
> + int i;
> +
> + nvme_check_constraints(n, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> +
> + nvme_init_state(n);
> + nvme_init_blk(n, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + nvme_init_pci(n, pci_dev);
> + nvme_init_ctrl(n, pci_dev);
> +
> for (i = 0; i < n->num_namespaces; i++) {
> nvme_init_namespace(n, &n->namespaces[i], &err);
> if (err) {
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 09/16] nvme: factor out property/constraint checks
2020-04-15 13:01 ` [PATCH v2 09/16] nvme: factor out property/constraint checks Klaus Jensen
@ 2020-04-15 13:08 ` Philippe Mathieu-Daudé
2020-04-21 14:53 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:08 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
On 4/15/20 3:01 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 43 ++++++++++++++++++++++++++++---------------
> 1 file changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 44856e873fd1..5f9ebbd6a1d5 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1311,24 +1311,19 @@ static const MemoryRegionOps nvme_cmb_ops = {
> },
> };
>
> -static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> +static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
> {
> - NvmeCtrl *n = NVME(pci_dev);
> - NvmeIdCtrl *id = &n->id_ctrl;
> + NvmeParams *params = &n->params;
>
> - int i;
> - int64_t bs_size;
> - uint8_t *pci_conf;
> -
> - if (n->params.num_queues) {
> + if (params->num_queues) {
> warn_report("num_queues is deprecated; please use max_ioqpairs "
> "instead");
>
> - n->params.max_ioqpairs = n->params.num_queues - 1;
> + params->max_ioqpairs = params->num_queues - 1;
> }
>
> - if (n->params.max_ioqpairs < 1 ||
> - n->params.max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
> + if (params->max_ioqpairs < 1 ||
> + params->max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
> error_setg(errp, "max_ioqpairs must be between 1 and %d",
> PCI_MSIX_FLAGS_QSIZE);
> return;
> @@ -1339,16 +1334,34 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> return;
> }
>
> + if (!params->serial) {
> + error_setg(errp, "serial property not set");
> + return;
> + }
> +}
> +
> +static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> +{
> + NvmeCtrl *n = NVME(pci_dev);
> + NvmeIdCtrl *id = &n->id_ctrl;
> + Error *err = NULL;
Nitpick if you ever have to respin, name this 'local_err'.
> +
> + int i;
> + int64_t bs_size;
> + uint8_t *pci_conf;
> +
> + nvme_check_constraints(n, &err);
> + if (err) {
> + error_propagate(errp, err);
Thanks :)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> + return;
> + }
> +
> bs_size = blk_getlength(n->conf.blk);
> if (bs_size < 0) {
> error_setg(errp, "could not get backing file size");
> return;
> }
>
> - if (!n->params.serial) {
> - error_setg(errp, "serial property not set");
> - return;
> - }
> blkconf_blocksizes(&n->conf);
> if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
> false, errp)) {
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 11/16] nvme: factor out block backend setup
2020-04-15 13:01 ` [PATCH v2 11/16] nvme: factor out block backend setup Klaus Jensen
@ 2020-04-15 13:08 ` Philippe Mathieu-Daudé
2020-04-21 15:04 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:08 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
On 4/15/20 3:01 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 45a352b63d89..80da0825d295 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1351,6 +1351,13 @@ static void nvme_init_state(NvmeCtrl *n)
> n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
> }
>
> +static void nvme_init_blk(NvmeCtrl *n, Error **errp)
> +{
> + blkconf_blocksizes(&n->conf);
> + blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
> + false, errp);
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> +}
> +
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> {
> NvmeCtrl *n = NVME(pci_dev);
> @@ -1375,9 +1382,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> return;
> }
>
> - blkconf_blocksizes(&n->conf);
> - if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
> - false, errp)) {
> + nvme_init_blk(n, &err);
> + if (err) {
> + error_propagate(errp, err);
> return;
> }
>
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 12/16] nvme: add namespace helpers
2020-04-15 13:01 ` [PATCH v2 12/16] nvme: add namespace helpers Klaus Jensen
@ 2020-04-15 13:09 ` Philippe Mathieu-Daudé
2020-04-21 15:41 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:09 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
On 4/15/20 3:01 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Introduce some small helpers to make the next patches easier on the eye.
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 3 +--
> hw/block/nvme.h | 16 ++++++++++++++++
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 80da0825d295..d5244102252c 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1469,8 +1469,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> id_ns->dps = 0;
> id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
> id_ns->ncap = id_ns->nuse = id_ns->nsze =
> - cpu_to_le64(n->ns_size >>
> - id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds);
> + cpu_to_le64(nvme_ns_nlbas(n, ns));
Thanks :)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> }
> }
>
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 7eecfd3a50f6..dd932a9e7ebc 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -67,6 +67,17 @@ typedef struct NvmeNamespace {
> NvmeIdNs id_ns;
> } NvmeNamespace;
>
> +static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
> +{
> + NvmeIdNs *id_ns = &ns->id_ns;
> + return &id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(id_ns->flbas)];
> +}
> +
> +static inline uint8_t nvme_ns_lbads(NvmeNamespace *ns)
> +{
> + return nvme_ns_lbaf(ns)->ds;
> +}
> +
> #define TYPE_NVME "nvme"
> #define NVME(obj) \
> OBJECT_CHECK(NvmeCtrl, (obj), TYPE_NVME)
> @@ -101,4 +112,9 @@ typedef struct NvmeCtrl {
> NvmeIdCtrl id_ctrl;
> } NvmeCtrl;
>
> +static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
> +{
> + return n->ns_size >> nvme_ns_lbads(ns);
> +}
> +
> #endif /* HW_NVME_H */
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 01/16] nvme: fix pci doorbell size calculation
2020-04-15 13:01 ` [PATCH v2 01/16] nvme: fix pci doorbell size calculation Klaus Jensen
@ 2020-04-15 13:13 ` Philippe Mathieu-Daudé
2020-04-21 9:39 ` Maxim Levitsky
0 siblings, 1 reply; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:13 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
On 4/15/20 3:01 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> The size of the BAR is 0x1000 (main registers) + 8 bytes for each
> queue. Currently, the size of the BAR is calculated like so:
>
> n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
>
> Since the 'num_queues' parameter already accounts for the admin queue,
> this should in any case not need to be incremented by one. Also, the
> size should be initialized to (0x1000).
>
> n->reg_size = pow2ceil(0x1000 + 2 * n->num_queues * 4);
>
> This, with the default value of num_queues (64), we will set aside room
> for 1 admin queue and 63 I/O queues (4 bytes per doorbell, 2 doorbells
> per queue).
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/nvme.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index d28335cbf377..5b5f75c9d29e 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -43,6 +43,9 @@
> #include "trace.h"
> #include "nvme.h"
>
> +#define NVME_REG_SIZE 0x1000
> +#define NVME_DB_SIZE 4
> +
> #define NVME_GUEST_ERR(trace, fmt, ...) \
> do { \
> (trace_##trace)(__VA_ARGS__); \
> @@ -1345,7 +1348,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> pcie_endpoint_cap_init(pci_dev, 0x80);
>
> n->num_namespaces = 1;
> - n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
> +
> + /* num_queues is really number of pairs, so each has two doorbells */
> + n->reg_size = pow2ceil(NVME_REG_SIZE + 2 * n->num_queues * NVME_DB_SIZE);
Unrelated to this change, but it would be cleaner to initialize reg_size
using MAX_NUM_QUEUES, then in the I/O handler log GUEST_ERROR when
registers > n->num_queues accessed. This would model closer to the hardware.
> n->ns_size = bs_size / (uint64_t)n->num_namespaces;
>
> n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 14/16] nvme: factor out pci setup
2020-04-15 13:01 ` [PATCH v2 14/16] nvme: factor out pci setup Klaus Jensen
@ 2020-04-15 13:14 ` Philippe Mathieu-Daudé
2020-04-21 15:59 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:14 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
On 4/15/20 3:01 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 30 ++++++++++++++++++------------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 2b007115c302..906ae595025a 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1379,6 +1379,22 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
> id_ns->nuse = id_ns->ncap;
> }
>
> +static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> +{
> + uint8_t *pci_conf = pci_dev->config;
> +
> + pci_conf[PCI_INTERRUPT_PIN] = 1;
> + pci_config_set_prog_interface(pci_conf, 0x2);
> + pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
> + pcie_endpoint_cap_init(pci_dev, 0x80);
> +
> + memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n, "nvme",
> + n->reg_size);
> + pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
> + PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem);
> + msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
> +}
> +
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> {
> NvmeCtrl *n = NVME(pci_dev);
> @@ -1402,19 +1418,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> return;
> }
>
> + nvme_init_pci(n, pci_dev);
> +
> pci_conf = pci_dev->config;
> - pci_conf[PCI_INTERRUPT_PIN] = 1;
> - pci_config_set_prog_interface(pci_dev->config, 0x2);
> - pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
> - pcie_endpoint_cap_init(pci_dev, 0x80);
> -
> - memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
> - "nvme", n->reg_size);
> - pci_register_bar(pci_dev, 0,
> - PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
> - &n->iomem);
> - msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
> -
> id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
> id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
> strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 13/16] nvme: factor out namespace setup
2020-04-15 13:01 ` [PATCH v2 13/16] nvme: factor out namespace setup Klaus Jensen
@ 2020-04-15 13:16 ` Philippe Mathieu-Daudé
2020-04-15 13:20 ` Klaus Birkelund Jensen
2020-04-21 15:57 ` Maxim Levitsky
1 sibling, 1 reply; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:16 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
On 4/15/20 3:01 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 46 ++++++++++++++++++++++++++--------------------
> 1 file changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index d5244102252c..2b007115c302 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1358,6 +1358,27 @@ static void nvme_init_blk(NvmeCtrl *n, Error **errp)
> false, errp);
> }
>
> +static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
> +{
> + int64_t bs_size;
> + NvmeIdNs *id_ns = &ns->id_ns;
> +
> + bs_size = blk_getlength(n->conf.blk);
> + if (bs_size < 0) {
> + error_setg_errno(errp, -bs_size, "could not get backing file size");
> + return;
> + }
> +
> + n->ns_size = bs_size;
> +
> + id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
> + id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(n, ns));
> +
> + /* no thin provisioning */
> + id_ns->ncap = id_ns->nsze;
> + id_ns->nuse = id_ns->ncap;
> +}
> +
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> {
> NvmeCtrl *n = NVME(pci_dev);
> @@ -1365,7 +1386,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> Error *err = NULL;
>
> int i;
> - int64_t bs_size;
> uint8_t *pci_conf;
>
> nvme_check_constraints(n, &err);
> @@ -1376,12 +1396,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>
> nvme_init_state(n);
>
> - bs_size = blk_getlength(n->conf.blk);
> - if (bs_size < 0) {
> - error_setg(errp, "could not get backing file size");
> - return;
> - }
> -
> nvme_init_blk(n, &err);
> if (err) {
> error_propagate(errp, err);
> @@ -1394,8 +1408,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
> pcie_endpoint_cap_init(pci_dev, 0x80);
>
> - n->ns_size = bs_size / (uint64_t)n->num_namespaces;
Valid because currently 'n->num_namespaces' = 1, OK.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
> "nvme", n->reg_size);
> pci_register_bar(pci_dev, 0,
> @@ -1459,17 +1471,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> }
>
> for (i = 0; i < n->num_namespaces; i++) {
> - NvmeNamespace *ns = &n->namespaces[i];
> - NvmeIdNs *id_ns = &ns->id_ns;
> - id_ns->nsfeat = 0;
> - id_ns->nlbaf = 0;
> - id_ns->flbas = 0;
> - id_ns->mc = 0;
> - id_ns->dpc = 0;
> - id_ns->dps = 0;
> - id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
> - id_ns->ncap = id_ns->nuse = id_ns->nsze =
> - cpu_to_le64(nvme_ns_nlbas(n, ns));
> + nvme_init_namespace(n, &n->namespaces[i], &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> }
> }
>
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 13/16] nvme: factor out namespace setup
2020-04-15 13:16 ` Philippe Mathieu-Daudé
@ 2020-04-15 13:20 ` Klaus Birkelund Jensen
2020-04-15 13:26 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 48+ messages in thread
From: Klaus Birkelund Jensen @ 2020-04-15 13:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Beata Michalska, qemu-block, Klaus Jensen,
qemu-devel, Max Reitz, Keith Busch, Javier Gonzalez,
Maxim Levitsky
On Apr 15 15:16, Philippe Mathieu-Daudé wrote:
> On 4/15/20 3:01 PM, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> >
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> > hw/block/nvme.c | 46 ++++++++++++++++++++++++++--------------------
> > 1 file changed, 26 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index d5244102252c..2b007115c302 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -1358,6 +1358,27 @@ static void nvme_init_blk(NvmeCtrl *n, Error **errp)
> > false, errp);
> > }
> > +static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
> > +{
> > + int64_t bs_size;
> > + NvmeIdNs *id_ns = &ns->id_ns;
> > +
> > + bs_size = blk_getlength(n->conf.blk);
> > + if (bs_size < 0) {
> > + error_setg_errno(errp, -bs_size, "could not get backing file size");
> > + return;
> > + }
> > +
> > + n->ns_size = bs_size;
> > +
> > + id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
> > + id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(n, ns));
> > +
> > + /* no thin provisioning */
> > + id_ns->ncap = id_ns->nsze;
> > + id_ns->nuse = id_ns->ncap;
> > +}
> > +
> > static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> > {
> > NvmeCtrl *n = NVME(pci_dev);
> > @@ -1365,7 +1386,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> > Error *err = NULL;
> > int i;
> > - int64_t bs_size;
> > uint8_t *pci_conf;
> > nvme_check_constraints(n, &err);
> > @@ -1376,12 +1396,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> > nvme_init_state(n);
> > - bs_size = blk_getlength(n->conf.blk);
> > - if (bs_size < 0) {
> > - error_setg(errp, "could not get backing file size");
> > - return;
> > - }
> > -
> > nvme_init_blk(n, &err);
> > if (err) {
> > error_propagate(errp, err);
> > @@ -1394,8 +1408,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> > pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
> > pcie_endpoint_cap_init(pci_dev, 0x80);
> > - n->ns_size = bs_size / (uint64_t)n->num_namespaces;
>
> Valid because currently 'n->num_namespaces' = 1, OK.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Thank you for the reviews Philippe and the suggesting that I split up
the series :)
I'll get the v1.3 series ready next.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 13/16] nvme: factor out namespace setup
2020-04-15 13:20 ` Klaus Birkelund Jensen
@ 2020-04-15 13:26 ` Philippe Mathieu-Daudé
2020-04-16 6:03 ` Klaus Birkelund Jensen
0 siblings, 1 reply; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-15 13:26 UTC (permalink / raw)
To: Klaus Birkelund Jensen
Cc: Kevin Wolf, Beata Michalska, qemu-block, Klaus Jensen,
qemu-devel, Max Reitz, Keith Busch, Javier Gonzalez,
Maxim Levitsky
On 4/15/20 3:20 PM, Klaus Birkelund Jensen wrote:
> On Apr 15 15:16, Philippe Mathieu-Daudé wrote:
>> On 4/15/20 3:01 PM, Klaus Jensen wrote:
>>> From: Klaus Jensen <k.jensen@samsung.com>
>>>
>>> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
>>> ---
>>> hw/block/nvme.c | 46 ++++++++++++++++++++++++++--------------------
>>> 1 file changed, 26 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>>> index d5244102252c..2b007115c302 100644
>>> --- a/hw/block/nvme.c
>>> +++ b/hw/block/nvme.c
>>> @@ -1358,6 +1358,27 @@ static void nvme_init_blk(NvmeCtrl *n, Error **errp)
>>> false, errp);
>>> }
>>> +static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
>>> +{
>>> + int64_t bs_size;
>>> + NvmeIdNs *id_ns = &ns->id_ns;
>>> +
>>> + bs_size = blk_getlength(n->conf.blk);
>>> + if (bs_size < 0) {
>>> + error_setg_errno(errp, -bs_size, "could not get backing file size");
>>> + return;
>>> + }
>>> +
>>> + n->ns_size = bs_size;
>>> +
>>> + id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
>>> + id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(n, ns));
>>> +
>>> + /* no thin provisioning */
>>> + id_ns->ncap = id_ns->nsze;
>>> + id_ns->nuse = id_ns->ncap;
>>> +}
>>> +
>>> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>>> {
>>> NvmeCtrl *n = NVME(pci_dev);
>>> @@ -1365,7 +1386,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>>> Error *err = NULL;
>>> int i;
>>> - int64_t bs_size;
>>> uint8_t *pci_conf;
>>> nvme_check_constraints(n, &err);
>>> @@ -1376,12 +1396,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>>> nvme_init_state(n);
>>> - bs_size = blk_getlength(n->conf.blk);
>>> - if (bs_size < 0) {
>>> - error_setg(errp, "could not get backing file size");
>>> - return;
>>> - }
>>> -
>>> nvme_init_blk(n, &err);
>>> if (err) {
>>> error_propagate(errp, err);
>>> @@ -1394,8 +1408,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>>> pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
>>> pcie_endpoint_cap_init(pci_dev, 0x80);
>>> - n->ns_size = bs_size / (uint64_t)n->num_namespaces;
>>
>> Valid because currently 'n->num_namespaces' = 1, OK.
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>
> Thank you for the reviews Philippe and the suggesting that I split up
> the series :)
No prob, thanks for the quick fixes. Let's see if Keith is happy with
v2. Personally I don't have anymore comments.
>
> I'll get the v1.3 series ready next.
>
Cool. What really matters (to me) is seeing tests. If we can merge tests
(without multiple namespaces) before the rest of your series, even
better. Tests give reviewers/maintainers confidence that code isn't
breaking ;)
Regards,
Phil.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 00/16] nvme: refactoring and cleanups
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (15 preceding siblings ...)
2020-04-15 13:01 ` [PATCH v2 16/16] nvme: factor out controller identify setup Klaus Jensen
@ 2020-04-15 14:29 ` no-reply
2020-04-20 5:14 ` Klaus Birkelund Jensen
` (2 subsequent siblings)
19 siblings, 0 replies; 48+ messages in thread
From: no-reply @ 2020-04-15 14:29 UTC (permalink / raw)
To: its
Cc: kwolf, beata.michalska, qemu-block, k.jensen, qemu-devel, mreitz,
kbusch, its, javier.gonz, mlevitsk, philmd
Patchew URL: https://patchew.org/QEMU/20200415130159.611361-1-its@irrelevant.dk/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH v2 00/16] nvme: refactoring and cleanups
Message-id: 20200415130159.611361-1-its@irrelevant.dk
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
a0e808d nvme: factor out controller identify setup
ea2d295 nvme: factor out cmb setup
c3c1e51 nvme: factor out pci setup
b4b6c55 nvme: factor out namespace setup
79229ae nvme: add namespace helpers
c7d0f2b nvme: factor out block backend setup
0802218 nvme: factor out device state setup
b407ffa nvme: factor out property/constraint checks
a7b4ac0 nvme: remove redundant cmbloc/cmbsz members
bf8d4cc nvme: add max_ioqpairs device parameter
eef1607 nvme: refactor nvme_addr_read
c063b77 nvme: use constants in identify
9c1ad75 nvme: move device parameters to separate struct
72124cf nvme: remove superfluous breaks
8cbaf9e nvme: rename trace events to pci_nvme
826b0ca nvme: fix pci doorbell size calculation
=== OUTPUT BEGIN ===
1/16 Checking commit 826b0caf1bed (nvme: fix pci doorbell size calculation)
2/16 Checking commit 8cbaf9e98e00 (nvme: rename trace events to pci_nvme)
3/16 Checking commit 72124cfc8e35 (nvme: remove superfluous breaks)
4/16 Checking commit 9c1ad75817e7 (nvme: move device parameters to separate struct)
ERROR: Macros with complex values should be enclosed in parenthesis
#182: FILE: hw/block/nvme.h:6:
+#define DEFINE_NVME_PROPERTIES(_state, _props) \
+ DEFINE_PROP_STRING("serial", _state, _props.serial), \
+ DEFINE_PROP_UINT32("cmb_size_mb", _state, _props.cmb_size_mb, 0), \
+ DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64)
total: 1 errors, 0 warnings, 182 lines checked
Patch 4/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/16 Checking commit c063b77e218e (nvme: use constants in identify)
6/16 Checking commit eef16074638e (nvme: refactor nvme_addr_read)
7/16 Checking commit bf8d4cc67458 (nvme: add max_ioqpairs device parameter)
8/16 Checking commit a7b4ac0a9cbe (nvme: remove redundant cmbloc/cmbsz members)
9/16 Checking commit b407ffae89f8 (nvme: factor out property/constraint checks)
10/16 Checking commit 0802218ca18b (nvme: factor out device state setup)
11/16 Checking commit c7d0f2b17c08 (nvme: factor out block backend setup)
12/16 Checking commit 79229aef5988 (nvme: add namespace helpers)
13/16 Checking commit b4b6c55cd5af (nvme: factor out namespace setup)
14/16 Checking commit c3c1e5121db6 (nvme: factor out pci setup)
15/16 Checking commit ea2d29507c1e (nvme: factor out cmb setup)
16/16 Checking commit a0e808d2fca5 (nvme: factor out controller identify setup)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200415130159.611361-1-its@irrelevant.dk/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 13/16] nvme: factor out namespace setup
2020-04-15 13:26 ` Philippe Mathieu-Daudé
@ 2020-04-16 6:03 ` Klaus Birkelund Jensen
2020-04-24 10:13 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 48+ messages in thread
From: Klaus Birkelund Jensen @ 2020-04-16 6:03 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Beata Michalska, qemu-block, Klaus Jensen,
qemu-devel, Max Reitz, Keith Busch, Javier Gonzalez,
Maxim Levitsky
On Apr 15 15:26, Philippe Mathieu-Daudé wrote:
> On 4/15/20 3:20 PM, Klaus Birkelund Jensen wrote:
> >
> > I'll get the v1.3 series ready next.
> >
>
> Cool. What really matters (to me) is seeing tests. If we can merge tests
> (without multiple namespaces) before the rest of your series, even better.
> Tests give reviewers/maintainers confidence that code isn't breaking ;)
>
The patches that I contribute have been pretty extensively tested by
various means in a "host setting" (e.g. blktests and some internal
tools), which really exercise the device by doing heavy I/O, testing for
compliance and also just being mean to it (e.g. tripping bus mastering
while doing I/O).
Don't misunderstand me as trying to weasel my way out of writing tests,
but I just want to understand the scope of the tests that you are
looking for? I believe (hope!) that you are not asking me to implement a
user-space NVMe driver in the test, so I assume the tests should varify
more low level details?
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 00/16] nvme: refactoring and cleanups
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (16 preceding siblings ...)
2020-04-15 14:29 ` [PATCH v2 00/16] nvme: refactoring and cleanups no-reply
@ 2020-04-20 5:14 ` Klaus Birkelund Jensen
2020-04-20 17:38 ` Keith Busch
2020-04-21 16:24 ` Maxim Levitsky
19 siblings, 0 replies; 48+ messages in thread
From: Klaus Birkelund Jensen @ 2020-04-20 5:14 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Apr 15 15:01, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Changes since v1
> ~~~~~~~~~~~~~~~~
> * nvme: fix pci doorbell size calculation
> - added some defines and a better comment (Philippe)
>
> * nvme: rename trace events to pci_nvme
> - changed the prefix from nvme_dev to pci_nvme (Philippe)
>
> * nvme: add max_ioqpairs device parameter
> - added a deprecation comment. I doubt this will go in until 5.1, so
> changed it to "deprecated from 5.1" (Philippe)
>
> * nvme: factor out property/constraint checks
> * nvme: factor out block backend setup
> - changed to return void and propagate errors in proper QEMU style
> (Philippe)
>
> * nvme: add namespace helpers
> - use the helper immediately (Philippe)
>
> * nvme: factor out pci setup
> - removed setting of vendor and device id which is already inherited
> from nvme_class_init() (Philippe)
>
> * nvme: factor out cmb setup
> - add lost comment (Philippe)
>
>
> Klaus Jensen (16):
> nvme: fix pci doorbell size calculation
> nvme: rename trace events to pci_nvme
> nvme: remove superfluous breaks
> nvme: move device parameters to separate struct
> nvme: use constants in identify
> nvme: refactor nvme_addr_read
> nvme: add max_ioqpairs device parameter
> nvme: remove redundant cmbloc/cmbsz members
> nvme: factor out property/constraint checks
> nvme: factor out device state setup
> nvme: factor out block backend setup
> nvme: add namespace helpers
> nvme: factor out namespace setup
> nvme: factor out pci setup
> nvme: factor out cmb setup
> nvme: factor out controller identify setup
>
> hw/block/nvme.c | 433 ++++++++++++++++++++++++------------------
> hw/block/nvme.h | 36 +++-
> hw/block/trace-events | 172 ++++++++---------
> include/block/nvme.h | 8 +
> 4 files changed, 372 insertions(+), 277 deletions(-)
>
> --
> 2.26.0
>
Hi Keith,
You have acked most of this previously, but not in it's most recent
state. Since a good bunch of the refactoring patches have been split up
and changed, only a small subset of the patches still carry your
Acked-by.
The 'nvme: fix pci doorbell size calculation' and 'nvme: add
max_ioqpairs device parameter' are new since your ack and given their
nature a review from you would be nice :)
Thanks,
Klaus
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 00/16] nvme: refactoring and cleanups
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (17 preceding siblings ...)
2020-04-20 5:14 ` Klaus Birkelund Jensen
@ 2020-04-20 17:38 ` Keith Busch
2020-04-21 6:38 ` Klaus Birkelund Jensen
2020-04-21 16:24 ` Maxim Levitsky
19 siblings, 1 reply; 48+ messages in thread
From: Keith Busch @ 2020-04-20 17:38 UTC (permalink / raw)
To: Klaus Jensen
Cc: Kevin Wolf, Beata Michalska, qemu-block, Klaus Jensen,
qemu-devel, Max Reitz, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
The series looks good to me.
Reviewed-by: Keith Busch <kbusch@kernel.org>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 00/16] nvme: refactoring and cleanups
2020-04-20 17:38 ` Keith Busch
@ 2020-04-21 6:38 ` Klaus Birkelund Jensen
2020-04-21 15:47 ` Kevin Wolf
0 siblings, 1 reply; 48+ messages in thread
From: Klaus Birkelund Jensen @ 2020-04-21 6:38 UTC (permalink / raw)
To: Keith Busch
Cc: Kevin Wolf, Beata Michalska, qemu-block, Klaus Jensen,
qemu-devel, Max Reitz, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
On Apr 21 02:38, Keith Busch wrote:
> The series looks good to me.
>
> Reviewed-by: Keith Busch <kbusch@kernel.org>
Thanks for the review Keith!
Kevin, should I rebase this on block-next? I think it might have some
conflicts with the PMR patch that went in previously.
Philippe, then I can also change the *err to *local_err ;)
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 01/16] nvme: fix pci doorbell size calculation
2020-04-15 13:13 ` Philippe Mathieu-Daudé
@ 2020-04-21 9:39 ` Maxim Levitsky
0 siblings, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 9:39 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez
On Wed, 2020-04-15 at 15:13 +0200, Philippe Mathieu-Daudé wrote:
> On 4/15/20 3:01 PM, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> >
> > The size of the BAR is 0x1000 (main registers) + 8 bytes for each
> > queue. Currently, the size of the BAR is calculated like so:
> >
> > n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
> >
> > Since the 'num_queues' parameter already accounts for the admin queue,
> > this should in any case not need to be incremented by one. Also, the
> > size should be initialized to (0x1000).
> >
> > n->reg_size = pow2ceil(0x1000 + 2 * n->num_queues * 4);
> >
> > This, with the default value of num_queues (64), we will set aside room
> > for 1 admin queue and 63 I/O queues (4 bytes per doorbell, 2 doorbells
> > per queue).
> >
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > hw/block/nvme.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index d28335cbf377..5b5f75c9d29e 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -43,6 +43,9 @@
> > #include "trace.h"
> > #include "nvme.h"
> >
> > +#define NVME_REG_SIZE 0x1000
> > +#define NVME_DB_SIZE 4
> > +
> > #define NVME_GUEST_ERR(trace, fmt, ...) \
> > do { \
> > (trace_##trace)(__VA_ARGS__); \
> > @@ -1345,7 +1348,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> > pcie_endpoint_cap_init(pci_dev, 0x80);
> >
> > n->num_namespaces = 1;
> > - n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
> > +
> > + /* num_queues is really number of pairs, so each has two doorbells */
> > + n->reg_size = pow2ceil(NVME_REG_SIZE + 2 * n->num_queues * NVME_DB_SIZE);
>
> Unrelated to this change, but it would be cleaner to initialize reg_size
> using MAX_NUM_QUEUES, then in the I/O handler log GUEST_ERROR when
> registers > n->num_queues accessed. This would model closer to the hardware.
Agree.
Also keep in mind that NVME_DB_SIZE is configurable by setting the doorbell stride.
(but this is optional, so currently this code is OK)
Other than that,
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
>
> > n->ns_size = bs_size / (uint64_t)n->num_namespaces;
> >
> > n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
> >
>
>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 02/16] nvme: rename trace events to pci_nvme
2020-04-15 13:01 ` [PATCH v2 02/16] nvme: rename trace events to pci_nvme Klaus Jensen
2020-04-15 13:04 ` Philippe Mathieu-Daudé
@ 2020-04-21 9:43 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 9:43 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Change the prefix of all nvme device related trace events to 'pci_nvme'
> to not clash with trace events from the nvme block driver.
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 190 +++++++++++++++++++++---------------------
> hw/block/trace-events | 172 +++++++++++++++++++-------------------
> 2 files changed, 180 insertions(+), 182 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 5b5f75c9d29e..931ddeb26ba0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -115,16 +115,16 @@ static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
> {
> if (cq->irq_enabled) {
> if (msix_enabled(&(n->parent_obj))) {
> - trace_nvme_irq_msix(cq->vector);
> + trace_pci_nvme_irq_msix(cq->vector);
> msix_notify(&(n->parent_obj), cq->vector);
> } else {
> - trace_nvme_irq_pin();
> + trace_pci_nvme_irq_pin();
> assert(cq->cqid < 64);
> n->irq_status |= 1 << cq->cqid;
> nvme_irq_check(n);
> }
> } else {
> - trace_nvme_irq_masked();
> + trace_pci_nvme_irq_masked();
> }
> }
>
> @@ -149,7 +149,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
> int num_prps = (len >> n->page_bits) + 1;
>
> if (unlikely(!prp1)) {
> - trace_nvme_err_invalid_prp();
> + trace_pci_nvme_err_invalid_prp();
> return NVME_INVALID_FIELD | NVME_DNR;
> } else if (n->cmbsz && prp1 >= n->ctrl_mem.addr &&
> prp1 < n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size)) {
> @@ -163,7 +163,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
> len -= trans_len;
> if (len) {
> if (unlikely(!prp2)) {
> - trace_nvme_err_invalid_prp2_missing();
> + trace_pci_nvme_err_invalid_prp2_missing();
> goto unmap;
> }
> if (len > n->page_size) {
> @@ -179,7 +179,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
>
> if (i == n->max_prp_ents - 1 && len > n->page_size) {
> if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
> - trace_nvme_err_invalid_prplist_ent(prp_ent);
> + trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
> goto unmap;
> }
>
> @@ -192,7 +192,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
> }
>
> if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
> - trace_nvme_err_invalid_prplist_ent(prp_ent);
> + trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
> goto unmap;
> }
>
> @@ -207,7 +207,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
> }
> } else {
> if (unlikely(prp2 & (n->page_size - 1))) {
> - trace_nvme_err_invalid_prp2_align(prp2);
> + trace_pci_nvme_err_invalid_prp2_align(prp2);
> goto unmap;
> }
> if (qsg->nsg) {
> @@ -255,20 +255,20 @@ static uint16_t nvme_dma_read_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len,
> QEMUIOVector iov;
> uint16_t status = NVME_SUCCESS;
>
> - trace_nvme_dma_read(prp1, prp2);
> + trace_pci_nvme_dma_read(prp1, prp2);
>
> if (nvme_map_prp(&qsg, &iov, prp1, prp2, len, n)) {
> return NVME_INVALID_FIELD | NVME_DNR;
> }
> if (qsg.nsg > 0) {
> if (unlikely(dma_buf_read(ptr, len, &qsg))) {
> - trace_nvme_err_invalid_dma();
> + trace_pci_nvme_err_invalid_dma();
> status = NVME_INVALID_FIELD | NVME_DNR;
> }
> qemu_sglist_destroy(&qsg);
> } else {
> if (unlikely(qemu_iovec_from_buf(&iov, 0, ptr, len) != len)) {
> - trace_nvme_err_invalid_dma();
> + trace_pci_nvme_err_invalid_dma();
> status = NVME_INVALID_FIELD | NVME_DNR;
> }
> qemu_iovec_destroy(&iov);
> @@ -357,7 +357,7 @@ static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd,
> uint32_t count = nlb << data_shift;
>
> if (unlikely(slba + nlb > ns->id_ns.nsze)) {
> - trace_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
> + trace_pci_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
> return NVME_LBA_RANGE | NVME_DNR;
> }
>
> @@ -385,11 +385,11 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd,
> int is_write = rw->opcode == NVME_CMD_WRITE ? 1 : 0;
> enum BlockAcctType acct = is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ;
>
> - trace_nvme_rw(is_write ? "write" : "read", nlb, data_size, slba);
> + trace_pci_nvme_rw(is_write ? "write" : "read", nlb, data_size, slba);
>
> if (unlikely((slba + nlb) > ns->id_ns.nsze)) {
> block_acct_invalid(blk_get_stats(n->conf.blk), acct);
> - trace_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
> + trace_pci_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
> return NVME_LBA_RANGE | NVME_DNR;
> }
>
> @@ -424,7 +424,7 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> uint32_t nsid = le32_to_cpu(cmd->nsid);
>
> if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
> - trace_nvme_err_invalid_ns(nsid, n->num_namespaces);
> + trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
> return NVME_INVALID_NSID | NVME_DNR;
> }
>
> @@ -438,7 +438,7 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> case NVME_CMD_READ:
> return nvme_rw(n, ns, cmd, req);
> default:
> - trace_nvme_err_invalid_opc(cmd->opcode);
> + trace_pci_nvme_err_invalid_opc(cmd->opcode);
> return NVME_INVALID_OPCODE | NVME_DNR;
> }
> }
> @@ -463,11 +463,11 @@ static uint16_t nvme_del_sq(NvmeCtrl *n, NvmeCmd *cmd)
> uint16_t qid = le16_to_cpu(c->qid);
>
> if (unlikely(!qid || nvme_check_sqid(n, qid))) {
> - trace_nvme_err_invalid_del_sq(qid);
> + trace_pci_nvme_err_invalid_del_sq(qid);
> return NVME_INVALID_QID | NVME_DNR;
> }
>
> - trace_nvme_del_sq(qid);
> + trace_pci_nvme_del_sq(qid);
>
> sq = n->sq[qid];
> while (!QTAILQ_EMPTY(&sq->out_req_list)) {
> @@ -531,26 +531,26 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeCmd *cmd)
> uint16_t qflags = le16_to_cpu(c->sq_flags);
> uint64_t prp1 = le64_to_cpu(c->prp1);
>
> - trace_nvme_create_sq(prp1, sqid, cqid, qsize, qflags);
> + trace_pci_nvme_create_sq(prp1, sqid, cqid, qsize, qflags);
>
> if (unlikely(!cqid || nvme_check_cqid(n, cqid))) {
> - trace_nvme_err_invalid_create_sq_cqid(cqid);
> + trace_pci_nvme_err_invalid_create_sq_cqid(cqid);
> return NVME_INVALID_CQID | NVME_DNR;
> }
> if (unlikely(!sqid || !nvme_check_sqid(n, sqid))) {
> - trace_nvme_err_invalid_create_sq_sqid(sqid);
> + trace_pci_nvme_err_invalid_create_sq_sqid(sqid);
> return NVME_INVALID_QID | NVME_DNR;
> }
> if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
> - trace_nvme_err_invalid_create_sq_size(qsize);
> + trace_pci_nvme_err_invalid_create_sq_size(qsize);
> return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
> }
> if (unlikely(!prp1 || prp1 & (n->page_size - 1))) {
> - trace_nvme_err_invalid_create_sq_addr(prp1);
> + trace_pci_nvme_err_invalid_create_sq_addr(prp1);
> return NVME_INVALID_FIELD | NVME_DNR;
> }
> if (unlikely(!(NVME_SQ_FLAGS_PC(qflags)))) {
> - trace_nvme_err_invalid_create_sq_qflags(NVME_SQ_FLAGS_PC(qflags));
> + trace_pci_nvme_err_invalid_create_sq_qflags(NVME_SQ_FLAGS_PC(qflags));
> return NVME_INVALID_FIELD | NVME_DNR;
> }
> sq = g_malloc0(sizeof(*sq));
> @@ -576,17 +576,17 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeCmd *cmd)
> uint16_t qid = le16_to_cpu(c->qid);
>
> if (unlikely(!qid || nvme_check_cqid(n, qid))) {
> - trace_nvme_err_invalid_del_cq_cqid(qid);
> + trace_pci_nvme_err_invalid_del_cq_cqid(qid);
> return NVME_INVALID_CQID | NVME_DNR;
> }
>
> cq = n->cq[qid];
> if (unlikely(!QTAILQ_EMPTY(&cq->sq_list))) {
> - trace_nvme_err_invalid_del_cq_notempty(qid);
> + trace_pci_nvme_err_invalid_del_cq_notempty(qid);
> return NVME_INVALID_QUEUE_DEL;
> }
> nvme_irq_deassert(n, cq);
> - trace_nvme_del_cq(qid);
> + trace_pci_nvme_del_cq(qid);
> nvme_free_cq(cq, n);
> return NVME_SUCCESS;
> }
> @@ -619,27 +619,27 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
> uint16_t qflags = le16_to_cpu(c->cq_flags);
> uint64_t prp1 = le64_to_cpu(c->prp1);
>
> - trace_nvme_create_cq(prp1, cqid, vector, qsize, qflags,
> - NVME_CQ_FLAGS_IEN(qflags) != 0);
> + trace_pci_nvme_create_cq(prp1, cqid, vector, qsize, qflags,
> + NVME_CQ_FLAGS_IEN(qflags) != 0);
>
> if (unlikely(!cqid || !nvme_check_cqid(n, cqid))) {
> - trace_nvme_err_invalid_create_cq_cqid(cqid);
> + trace_pci_nvme_err_invalid_create_cq_cqid(cqid);
> return NVME_INVALID_CQID | NVME_DNR;
> }
> if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
> - trace_nvme_err_invalid_create_cq_size(qsize);
> + trace_pci_nvme_err_invalid_create_cq_size(qsize);
> return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
> }
> if (unlikely(!prp1)) {
> - trace_nvme_err_invalid_create_cq_addr(prp1);
> + trace_pci_nvme_err_invalid_create_cq_addr(prp1);
> return NVME_INVALID_FIELD | NVME_DNR;
> }
> if (unlikely(vector > n->num_queues)) {
> - trace_nvme_err_invalid_create_cq_vector(vector);
> + trace_pci_nvme_err_invalid_create_cq_vector(vector);
> return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
> }
> if (unlikely(!(NVME_CQ_FLAGS_PC(qflags)))) {
> - trace_nvme_err_invalid_create_cq_qflags(NVME_CQ_FLAGS_PC(qflags));
> + trace_pci_nvme_err_invalid_create_cq_qflags(NVME_CQ_FLAGS_PC(qflags));
> return NVME_INVALID_FIELD | NVME_DNR;
> }
>
> @@ -654,7 +654,7 @@ static uint16_t nvme_identify_ctrl(NvmeCtrl *n, NvmeIdentify *c)
> uint64_t prp1 = le64_to_cpu(c->prp1);
> uint64_t prp2 = le64_to_cpu(c->prp2);
>
> - trace_nvme_identify_ctrl();
> + trace_pci_nvme_identify_ctrl();
>
> return nvme_dma_read_prp(n, (uint8_t *)&n->id_ctrl, sizeof(n->id_ctrl),
> prp1, prp2);
> @@ -667,10 +667,10 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeIdentify *c)
> uint64_t prp1 = le64_to_cpu(c->prp1);
> uint64_t prp2 = le64_to_cpu(c->prp2);
>
> - trace_nvme_identify_ns(nsid);
> + trace_pci_nvme_identify_ns(nsid);
>
> if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
> - trace_nvme_err_invalid_ns(nsid, n->num_namespaces);
> + trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
> return NVME_INVALID_NSID | NVME_DNR;
> }
>
> @@ -690,7 +690,7 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeIdentify *c)
> uint16_t ret;
> int i, j = 0;
>
> - trace_nvme_identify_nslist(min_nsid);
> + trace_pci_nvme_identify_nslist(min_nsid);
>
> list = g_malloc0(data_len);
> for (i = 0; i < n->num_namespaces; i++) {
> @@ -719,14 +719,14 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
> case 0x02:
> return nvme_identify_nslist(n, c);
> default:
> - trace_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
> + trace_pci_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
> return NVME_INVALID_FIELD | NVME_DNR;
> }
> }
>
> static inline void nvme_set_timestamp(NvmeCtrl *n, uint64_t ts)
> {
> - trace_nvme_setfeat_timestamp(ts);
> + trace_pci_nvme_setfeat_timestamp(ts);
>
> n->host_timestamp = le64_to_cpu(ts);
> n->timestamp_set_qemu_clock_ms = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> @@ -759,7 +759,7 @@ static inline uint64_t nvme_get_timestamp(const NvmeCtrl *n)
> /* If the host timestamp is non-zero, set the timestamp origin */
> ts.origin = n->host_timestamp ? 0x01 : 0x00;
>
> - trace_nvme_getfeat_timestamp(ts.all);
> + trace_pci_nvme_getfeat_timestamp(ts.all);
>
> return cpu_to_le64(ts.all);
> }
> @@ -783,17 +783,17 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> switch (dw10) {
> case NVME_VOLATILE_WRITE_CACHE:
> result = blk_enable_write_cache(n->conf.blk);
> - trace_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> + trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> break;
> case NVME_NUMBER_OF_QUEUES:
> result = cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
> - trace_nvme_getfeat_numq(result);
> + trace_pci_nvme_getfeat_numq(result);
> break;
> case NVME_TIMESTAMP:
> return nvme_get_feature_timestamp(n, cmd);
> break;
> default:
> - trace_nvme_err_invalid_getfeat(dw10);
> + trace_pci_nvme_err_invalid_getfeat(dw10);
> return NVME_INVALID_FIELD | NVME_DNR;
> }
>
> @@ -829,9 +829,9 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> blk_set_enable_write_cache(n->conf.blk, dw11 & 1);
> break;
> case NVME_NUMBER_OF_QUEUES:
> - trace_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
> - ((dw11 >> 16) & 0xFFFF) + 1,
> - n->num_queues - 1, n->num_queues - 1);
> + trace_pci_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
> + ((dw11 >> 16) & 0xFFFF) + 1,
> + n->num_queues - 1, n->num_queues - 1);
> req->cqe.result =
> cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
> break;
> @@ -841,7 +841,7 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> break;
>
> default:
> - trace_nvme_err_invalid_setfeat(dw10);
> + trace_pci_nvme_err_invalid_setfeat(dw10);
> return NVME_INVALID_FIELD | NVME_DNR;
> }
> return NVME_SUCCESS;
> @@ -865,7 +865,7 @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> case NVME_ADM_CMD_GET_FEATURES:
> return nvme_get_feature(n, cmd, req);
> default:
> - trace_nvme_err_invalid_admin_opc(cmd->opcode);
> + trace_pci_nvme_err_invalid_admin_opc(cmd->opcode);
> return NVME_INVALID_OPCODE | NVME_DNR;
> }
> }
> @@ -928,77 +928,77 @@ static int nvme_start_ctrl(NvmeCtrl *n)
> uint32_t page_size = 1 << page_bits;
>
> if (unlikely(n->cq[0])) {
> - trace_nvme_err_startfail_cq();
> + trace_pci_nvme_err_startfail_cq();
> return -1;
> }
> if (unlikely(n->sq[0])) {
> - trace_nvme_err_startfail_sq();
> + trace_pci_nvme_err_startfail_sq();
> return -1;
> }
> if (unlikely(!n->bar.asq)) {
> - trace_nvme_err_startfail_nbarasq();
> + trace_pci_nvme_err_startfail_nbarasq();
> return -1;
> }
> if (unlikely(!n->bar.acq)) {
> - trace_nvme_err_startfail_nbaracq();
> + trace_pci_nvme_err_startfail_nbaracq();
> return -1;
> }
> if (unlikely(n->bar.asq & (page_size - 1))) {
> - trace_nvme_err_startfail_asq_misaligned(n->bar.asq);
> + trace_pci_nvme_err_startfail_asq_misaligned(n->bar.asq);
> return -1;
> }
> if (unlikely(n->bar.acq & (page_size - 1))) {
> - trace_nvme_err_startfail_acq_misaligned(n->bar.acq);
> + trace_pci_nvme_err_startfail_acq_misaligned(n->bar.acq);
> return -1;
> }
> if (unlikely(NVME_CC_MPS(n->bar.cc) <
> NVME_CAP_MPSMIN(n->bar.cap))) {
> - trace_nvme_err_startfail_page_too_small(
> + trace_pci_nvme_err_startfail_page_too_small(
> NVME_CC_MPS(n->bar.cc),
> NVME_CAP_MPSMIN(n->bar.cap));
> return -1;
> }
> if (unlikely(NVME_CC_MPS(n->bar.cc) >
> NVME_CAP_MPSMAX(n->bar.cap))) {
> - trace_nvme_err_startfail_page_too_large(
> + trace_pci_nvme_err_startfail_page_too_large(
> NVME_CC_MPS(n->bar.cc),
> NVME_CAP_MPSMAX(n->bar.cap));
> return -1;
> }
> if (unlikely(NVME_CC_IOCQES(n->bar.cc) <
> NVME_CTRL_CQES_MIN(n->id_ctrl.cqes))) {
> - trace_nvme_err_startfail_cqent_too_small(
> + trace_pci_nvme_err_startfail_cqent_too_small(
> NVME_CC_IOCQES(n->bar.cc),
> NVME_CTRL_CQES_MIN(n->bar.cap));
> return -1;
> }
> if (unlikely(NVME_CC_IOCQES(n->bar.cc) >
> NVME_CTRL_CQES_MAX(n->id_ctrl.cqes))) {
> - trace_nvme_err_startfail_cqent_too_large(
> + trace_pci_nvme_err_startfail_cqent_too_large(
> NVME_CC_IOCQES(n->bar.cc),
> NVME_CTRL_CQES_MAX(n->bar.cap));
> return -1;
> }
> if (unlikely(NVME_CC_IOSQES(n->bar.cc) <
> NVME_CTRL_SQES_MIN(n->id_ctrl.sqes))) {
> - trace_nvme_err_startfail_sqent_too_small(
> + trace_pci_nvme_err_startfail_sqent_too_small(
> NVME_CC_IOSQES(n->bar.cc),
> NVME_CTRL_SQES_MIN(n->bar.cap));
> return -1;
> }
> if (unlikely(NVME_CC_IOSQES(n->bar.cc) >
> NVME_CTRL_SQES_MAX(n->id_ctrl.sqes))) {
> - trace_nvme_err_startfail_sqent_too_large(
> + trace_pci_nvme_err_startfail_sqent_too_large(
> NVME_CC_IOSQES(n->bar.cc),
> NVME_CTRL_SQES_MAX(n->bar.cap));
> return -1;
> }
> if (unlikely(!NVME_AQA_ASQS(n->bar.aqa))) {
> - trace_nvme_err_startfail_asqent_sz_zero();
> + trace_pci_nvme_err_startfail_asqent_sz_zero();
> return -1;
> }
> if (unlikely(!NVME_AQA_ACQS(n->bar.aqa))) {
> - trace_nvme_err_startfail_acqent_sz_zero();
> + trace_pci_nvme_err_startfail_acqent_sz_zero();
> return -1;
> }
>
> @@ -1021,14 +1021,14 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
> unsigned size)
> {
> if (unlikely(offset & (sizeof(uint32_t) - 1))) {
> - NVME_GUEST_ERR(nvme_ub_mmiowr_misaligned32,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_misaligned32,
> "MMIO write not 32-bit aligned,"
> " offset=0x%"PRIx64"", offset);
> /* should be ignored, fall through for now */
> }
>
> if (unlikely(size < sizeof(uint32_t))) {
> - NVME_GUEST_ERR(nvme_ub_mmiowr_toosmall,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_toosmall,
> "MMIO write smaller than 32-bits,"
> " offset=0x%"PRIx64", size=%u",
> offset, size);
> @@ -1038,32 +1038,30 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
> switch (offset) {
> case 0xc: /* INTMS */
> if (unlikely(msix_enabled(&(n->parent_obj)))) {
> - NVME_GUEST_ERR(nvme_ub_mmiowr_intmask_with_msix,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_intmask_with_msix,
> "undefined access to interrupt mask set"
> " when MSI-X is enabled");
> /* should be ignored, fall through for now */
> }
> n->bar.intms |= data & 0xffffffff;
> n->bar.intmc = n->bar.intms;
> - trace_nvme_mmio_intm_set(data & 0xffffffff,
> - n->bar.intmc);
> + trace_pci_nvme_mmio_intm_set(data & 0xffffffff, n->bar.intmc);
> nvme_irq_check(n);
> break;
> case 0x10: /* INTMC */
> if (unlikely(msix_enabled(&(n->parent_obj)))) {
> - NVME_GUEST_ERR(nvme_ub_mmiowr_intmask_with_msix,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_intmask_with_msix,
> "undefined access to interrupt mask clr"
> " when MSI-X is enabled");
> /* should be ignored, fall through for now */
> }
> n->bar.intms &= ~(data & 0xffffffff);
> n->bar.intmc = n->bar.intms;
> - trace_nvme_mmio_intm_clr(data & 0xffffffff,
> - n->bar.intmc);
> + trace_pci_nvme_mmio_intm_clr(data & 0xffffffff, n->bar.intmc);
> nvme_irq_check(n);
> break;
> case 0x14: /* CC */
> - trace_nvme_mmio_cfg(data & 0xffffffff);
> + trace_pci_nvme_mmio_cfg(data & 0xffffffff);
> /* Windows first sends data, then sends enable bit */
> if (!NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc) &&
> !NVME_CC_SHN(data) && !NVME_CC_SHN(n->bar.cc))
> @@ -1074,42 +1072,42 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
> if (NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc)) {
> n->bar.cc = data;
> if (unlikely(nvme_start_ctrl(n))) {
> - trace_nvme_err_startfail();
> + trace_pci_nvme_err_startfail();
> n->bar.csts = NVME_CSTS_FAILED;
> } else {
> - trace_nvme_mmio_start_success();
> + trace_pci_nvme_mmio_start_success();
> n->bar.csts = NVME_CSTS_READY;
> }
> } else if (!NVME_CC_EN(data) && NVME_CC_EN(n->bar.cc)) {
> - trace_nvme_mmio_stopped();
> + trace_pci_nvme_mmio_stopped();
> nvme_clear_ctrl(n);
> n->bar.csts &= ~NVME_CSTS_READY;
> }
> if (NVME_CC_SHN(data) && !(NVME_CC_SHN(n->bar.cc))) {
> - trace_nvme_mmio_shutdown_set();
> + trace_pci_nvme_mmio_shutdown_set();
> nvme_clear_ctrl(n);
> n->bar.cc = data;
> n->bar.csts |= NVME_CSTS_SHST_COMPLETE;
> } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(n->bar.cc)) {
> - trace_nvme_mmio_shutdown_cleared();
> + trace_pci_nvme_mmio_shutdown_cleared();
> n->bar.csts &= ~NVME_CSTS_SHST_COMPLETE;
> n->bar.cc = data;
> }
> break;
> case 0x1C: /* CSTS */
> if (data & (1 << 4)) {
> - NVME_GUEST_ERR(nvme_ub_mmiowr_ssreset_w1c_unsupported,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_ssreset_w1c_unsupported,
> "attempted to W1C CSTS.NSSRO"
> " but CAP.NSSRS is zero (not supported)");
> } else if (data != 0) {
> - NVME_GUEST_ERR(nvme_ub_mmiowr_ro_csts,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_ro_csts,
> "attempted to set a read only bit"
> " of controller status");
> }
> break;
> case 0x20: /* NSSR */
> if (data == 0x4E564D65) {
> - trace_nvme_ub_mmiowr_ssreset_unsupported();
> + trace_pci_nvme_ub_mmiowr_ssreset_unsupported();
> } else {
> /* The spec says that writes of other values have no effect */
> return;
> @@ -1117,35 +1115,35 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
> break;
> case 0x24: /* AQA */
> n->bar.aqa = data & 0xffffffff;
> - trace_nvme_mmio_aqattr(data & 0xffffffff);
> + trace_pci_nvme_mmio_aqattr(data & 0xffffffff);
> break;
> case 0x28: /* ASQ */
> n->bar.asq = data;
> - trace_nvme_mmio_asqaddr(data);
> + trace_pci_nvme_mmio_asqaddr(data);
> break;
> case 0x2c: /* ASQ hi */
> n->bar.asq |= data << 32;
> - trace_nvme_mmio_asqaddr_hi(data, n->bar.asq);
> + trace_pci_nvme_mmio_asqaddr_hi(data, n->bar.asq);
> break;
> case 0x30: /* ACQ */
> - trace_nvme_mmio_acqaddr(data);
> + trace_pci_nvme_mmio_acqaddr(data);
> n->bar.acq = data;
> break;
> case 0x34: /* ACQ hi */
> n->bar.acq |= data << 32;
> - trace_nvme_mmio_acqaddr_hi(data, n->bar.acq);
> + trace_pci_nvme_mmio_acqaddr_hi(data, n->bar.acq);
> break;
> case 0x38: /* CMBLOC */
> - NVME_GUEST_ERR(nvme_ub_mmiowr_cmbloc_reserved,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_cmbloc_reserved,
> "invalid write to reserved CMBLOC"
> " when CMBSZ is zero, ignored");
> return;
> case 0x3C: /* CMBSZ */
> - NVME_GUEST_ERR(nvme_ub_mmiowr_cmbsz_readonly,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_cmbsz_readonly,
> "invalid write to read only CMBSZ, ignored");
> return;
> default:
> - NVME_GUEST_ERR(nvme_ub_mmiowr_invalid,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiowr_invalid,
> "invalid MMIO write,"
> " offset=0x%"PRIx64", data=%"PRIx64"",
> offset, data);
> @@ -1160,12 +1158,12 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
> uint64_t val = 0;
>
> if (unlikely(addr & (sizeof(uint32_t) - 1))) {
> - NVME_GUEST_ERR(nvme_ub_mmiord_misaligned32,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiord_misaligned32,
> "MMIO read not 32-bit aligned,"
> " offset=0x%"PRIx64"", addr);
> /* should RAZ, fall through for now */
> } else if (unlikely(size < sizeof(uint32_t))) {
> - NVME_GUEST_ERR(nvme_ub_mmiord_toosmall,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiord_toosmall,
> "MMIO read smaller than 32-bits,"
> " offset=0x%"PRIx64"", addr);
> /* should RAZ, fall through for now */
> @@ -1174,7 +1172,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
> if (addr < sizeof(n->bar)) {
> memcpy(&val, ptr + addr, size);
> } else {
> - NVME_GUEST_ERR(nvme_ub_mmiord_invalid_ofs,
> + NVME_GUEST_ERR(pci_nvme_ub_mmiord_invalid_ofs,
> "MMIO read beyond last register,"
> " offset=0x%"PRIx64", returning 0", addr);
> }
> @@ -1187,7 +1185,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
> uint32_t qid;
>
> if (unlikely(addr & ((1 << 2) - 1))) {
> - NVME_GUEST_ERR(nvme_ub_db_wr_misaligned,
> + NVME_GUEST_ERR(pci_nvme_ub_db_wr_misaligned,
> "doorbell write not 32-bit aligned,"
> " offset=0x%"PRIx64", ignoring", addr);
> return;
> @@ -1202,7 +1200,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
>
> qid = (addr - (0x1000 + (1 << 2))) >> 3;
> if (unlikely(nvme_check_cqid(n, qid))) {
> - NVME_GUEST_ERR(nvme_ub_db_wr_invalid_cq,
> + NVME_GUEST_ERR(pci_nvme_ub_db_wr_invalid_cq,
> "completion queue doorbell write"
> " for nonexistent queue,"
> " sqid=%"PRIu32", ignoring", qid);
> @@ -1211,7 +1209,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
>
> cq = n->cq[qid];
> if (unlikely(new_head >= cq->size)) {
> - NVME_GUEST_ERR(nvme_ub_db_wr_invalid_cqhead,
> + NVME_GUEST_ERR(pci_nvme_ub_db_wr_invalid_cqhead,
> "completion queue doorbell write value"
> " beyond queue size, sqid=%"PRIu32","
> " new_head=%"PRIu16", ignoring",
> @@ -1240,7 +1238,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
>
> qid = (addr - 0x1000) >> 3;
> if (unlikely(nvme_check_sqid(n, qid))) {
> - NVME_GUEST_ERR(nvme_ub_db_wr_invalid_sq,
> + NVME_GUEST_ERR(pci_nvme_ub_db_wr_invalid_sq,
> "submission queue doorbell write"
> " for nonexistent queue,"
> " sqid=%"PRIu32", ignoring", qid);
> @@ -1249,7 +1247,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
>
> sq = n->sq[qid];
> if (unlikely(new_tail >= sq->size)) {
> - NVME_GUEST_ERR(nvme_ub_db_wr_invalid_sqtail,
> + NVME_GUEST_ERR(pci_nvme_ub_db_wr_invalid_sqtail,
> "submission queue doorbell write value"
> " beyond queue size, sqid=%"PRIu32","
> " new_tail=%"PRIu16", ignoring",
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index bf6d11b58b85..1725abd92121 100644
> --- a/hw/block/trace-events
> +++ b/hw/block/trace-events
> @@ -29,96 +29,96 @@ hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int t
>
> # nvme.c
> # nvme traces for successful events
> -nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u"
> -nvme_irq_pin(void) "pulsing IRQ pin"
> -nvme_irq_masked(void) "IRQ is masked"
> -nvme_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64" prp2=0x%"PRIx64""
> -nvme_rw(const char *verb, uint32_t blk_count, uint64_t byte_count, uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64""
> -nvme_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16",
> qflags=%"PRIu16""
> -nvme_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", cqid=%"PRIu16", vector=%"PRIu16", qsize=%"PRIu16",
> qflags=%"PRIu16", ien=%d"
> -nvme_del_sq(uint16_t qid) "deleting submission queue sqid=%"PRIu16""
> -nvme_del_cq(uint16_t cqid) "deleted completion queue, cqid=%"PRIu16""
> -nvme_identify_ctrl(void) "identify controller"
> -nvme_identify_ns(uint16_t ns) "identify namespace, nsid=%"PRIu16""
> -nvme_identify_nslist(uint16_t ns) "identify namespace list, nsid=%"PRIu16""
> -nvme_getfeat_vwcache(const char* result) "get feature volatile write cache, result=%s"
> -nvme_getfeat_numq(int result) "get feature number of queues, result=%d"
> -nvme_setfeat_numq(int reqcq, int reqsq, int gotcq, int gotsq) "requested cq_count=%d sq_count=%d, responding with cq_count=%d sq_count=%d"
> -nvme_setfeat_timestamp(uint64_t ts) "set feature timestamp = 0x%"PRIx64""
> -nvme_getfeat_timestamp(uint64_t ts) "get feature timestamp = 0x%"PRIx64""
> -nvme_mmio_intm_set(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask set, data=0x%"PRIx64", new_mask=0x%"PRIx64""
> -nvme_mmio_intm_clr(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask clr, data=0x%"PRIx64", new_mask=0x%"PRIx64""
> -nvme_mmio_cfg(uint64_t data) "wrote MMIO, config controller config=0x%"PRIx64""
> -nvme_mmio_aqattr(uint64_t data) "wrote MMIO, admin queue attributes=0x%"PRIx64""
> -nvme_mmio_asqaddr(uint64_t data) "wrote MMIO, admin submission queue address=0x%"PRIx64""
> -nvme_mmio_acqaddr(uint64_t data) "wrote MMIO, admin completion queue address=0x%"PRIx64""
> -nvme_mmio_asqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin submission queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
> -nvme_mmio_acqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin completion queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
> -nvme_mmio_start_success(void) "setting controller enable bit succeeded"
> -nvme_mmio_stopped(void) "cleared controller enable bit"
> -nvme_mmio_shutdown_set(void) "shutdown bit set"
> -nvme_mmio_shutdown_cleared(void) "shutdown bit cleared"
> +pci_nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u"
> +pci_nvme_irq_pin(void) "pulsing IRQ pin"
> +pci_nvme_irq_masked(void) "IRQ is masked"
> +pci_nvme_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64" prp2=0x%"PRIx64""
> +pci_nvme_rw(const char *verb, uint32_t blk_count, uint64_t byte_count, uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64""
> +pci_nvme_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16",
> qflags=%"PRIu16""
> +pci_nvme_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", cqid=%"PRIu16", vector=%"PRIu16",
> qsize=%"PRIu16", qflags=%"PRIu16", ien=%d"
> +pci_nvme_del_sq(uint16_t qid) "deleting submission queue sqid=%"PRIu16""
> +pci_nvme_del_cq(uint16_t cqid) "deleted completion queue, cqid=%"PRIu16""
> +pci_nvme_identify_ctrl(void) "identify controller"
> +pci_nvme_identify_ns(uint16_t ns) "identify namespace, nsid=%"PRIu16""
> +pci_nvme_identify_nslist(uint16_t ns) "identify namespace list, nsid=%"PRIu16""
> +pci_nvme_getfeat_vwcache(const char* result) "get feature volatile write cache, result=%s"
> +pci_nvme_getfeat_numq(int result) "get feature number of queues, result=%d"
> +pci_nvme_setfeat_numq(int reqcq, int reqsq, int gotcq, int gotsq) "requested cq_count=%d sq_count=%d, responding with cq_count=%d sq_count=%d"
> +pci_nvme_setfeat_timestamp(uint64_t ts) "set feature timestamp = 0x%"PRIx64""
> +pci_nvme_getfeat_timestamp(uint64_t ts) "get feature timestamp = 0x%"PRIx64""
> +pci_nvme_mmio_intm_set(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask set, data=0x%"PRIx64", new_mask=0x%"PRIx64""
> +pci_nvme_mmio_intm_clr(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask clr, data=0x%"PRIx64", new_mask=0x%"PRIx64""
> +pci_nvme_mmio_cfg(uint64_t data) "wrote MMIO, config controller config=0x%"PRIx64""
> +pci_nvme_mmio_aqattr(uint64_t data) "wrote MMIO, admin queue attributes=0x%"PRIx64""
> +pci_nvme_mmio_asqaddr(uint64_t data) "wrote MMIO, admin submission queue address=0x%"PRIx64""
> +pci_nvme_mmio_acqaddr(uint64_t data) "wrote MMIO, admin completion queue address=0x%"PRIx64""
> +pci_nvme_mmio_asqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin submission queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
> +pci_nvme_mmio_acqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin completion queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
> +pci_nvme_mmio_start_success(void) "setting controller enable bit succeeded"
> +pci_nvme_mmio_stopped(void) "cleared controller enable bit"
> +pci_nvme_mmio_shutdown_set(void) "shutdown bit set"
> +pci_nvme_mmio_shutdown_cleared(void) "shutdown bit cleared"
>
> # nvme traces for error conditions
> -nvme_err_invalid_dma(void) "PRP/SGL is too small for transfer size"
> -nvme_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null or not page aligned: 0x%"PRIx64""
> -nvme_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: 0x%"PRIx64""
> -nvme_err_invalid_prp2_missing(void) "PRP2 is null and more data to be transferred"
> -nvme_err_invalid_prp(void) "invalid PRP"
> -nvme_err_invalid_ns(uint32_t ns, uint32_t limit) "invalid namespace %u not within 1-%u"
> -nvme_err_invalid_opc(uint8_t opc) "invalid opcode 0x%"PRIx8""
> -nvme_err_invalid_admin_opc(uint8_t opc) "invalid admin opcode 0x%"PRIx8""
> -nvme_err_invalid_lba_range(uint64_t start, uint64_t len, uint64_t limit) "Invalid LBA start=%"PRIu64" len=%"PRIu64" limit=%"PRIu64""
> -nvme_err_invalid_del_sq(uint16_t qid) "invalid submission queue deletion, sid=%"PRIu16""
> -nvme_err_invalid_create_sq_cqid(uint16_t cqid) "failed creating submission queue, invalid cqid=%"PRIu16""
> -nvme_err_invalid_create_sq_sqid(uint16_t sqid) "failed creating submission queue, invalid sqid=%"PRIu16""
> -nvme_err_invalid_create_sq_size(uint16_t qsize) "failed creating submission queue, invalid qsize=%"PRIu16""
> -nvme_err_invalid_create_sq_addr(uint64_t addr) "failed creating submission queue, addr=0x%"PRIx64""
> -nvme_err_invalid_create_sq_qflags(uint16_t qflags) "failed creating submission queue, qflags=%"PRIu16""
> -nvme_err_invalid_del_cq_cqid(uint16_t cqid) "failed deleting completion queue, cqid=%"PRIu16""
> -nvme_err_invalid_del_cq_notempty(uint16_t cqid) "failed deleting completion queue, it is not empty, cqid=%"PRIu16""
> -nvme_err_invalid_create_cq_cqid(uint16_t cqid) "failed creating completion queue, cqid=%"PRIu16""
> -nvme_err_invalid_create_cq_size(uint16_t size) "failed creating completion queue, size=%"PRIu16""
> -nvme_err_invalid_create_cq_addr(uint64_t addr) "failed creating completion queue, addr=0x%"PRIx64""
> -nvme_err_invalid_create_cq_vector(uint16_t vector) "failed creating completion queue, vector=%"PRIu16""
> -nvme_err_invalid_create_cq_qflags(uint16_t qflags) "failed creating completion queue, qflags=%"PRIu16""
> -nvme_err_invalid_identify_cns(uint16_t cns) "identify, invalid cns=0x%"PRIx16""
> -nvme_err_invalid_getfeat(int dw10) "invalid get features, dw10=0x%"PRIx32""
> -nvme_err_invalid_setfeat(uint32_t dw10) "invalid set features, dw10=0x%"PRIx32""
> -nvme_err_startfail_cq(void) "nvme_start_ctrl failed because there are non-admin completion queues"
> -nvme_err_startfail_sq(void) "nvme_start_ctrl failed because there are non-admin submission queues"
> -nvme_err_startfail_nbarasq(void) "nvme_start_ctrl failed because the admin submission queue address is null"
> -nvme_err_startfail_nbaracq(void) "nvme_start_ctrl failed because the admin completion queue address is null"
> -nvme_err_startfail_asq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin submission queue address is misaligned: 0x%"PRIx64""
> -nvme_err_startfail_acq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin completion queue address is misaligned: 0x%"PRIx64""
> -nvme_err_startfail_page_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too small: log2size=%u, min=%u"
> -nvme_err_startfail_page_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too large: log2size=%u, max=%u"
> -nvme_err_startfail_cqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too small: log2size=%u, min=%u"
> -nvme_err_startfail_cqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too large: log2size=%u, max=%u"
> -nvme_err_startfail_sqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too small: log2size=%u, min=%u"
> -nvme_err_startfail_sqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too large: log2size=%u, max=%u"
> -nvme_err_startfail_asqent_sz_zero(void) "nvme_start_ctrl failed because the admin submission queue size is zero"
> -nvme_err_startfail_acqent_sz_zero(void) "nvme_start_ctrl failed because the admin completion queue size is zero"
> -nvme_err_startfail(void) "setting controller enable bit failed"
> +pci_nvme_err_invalid_dma(void) "PRP/SGL is too small for transfer size"
> +pci_nvme_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null or not page aligned: 0x%"PRIx64""
> +pci_nvme_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: 0x%"PRIx64""
> +pci_nvme_err_invalid_prp2_missing(void) "PRP2 is null and more data to be transferred"
> +pci_nvme_err_invalid_prp(void) "invalid PRP"
> +pci_nvme_err_invalid_ns(uint32_t ns, uint32_t limit) "invalid namespace %u not within 1-%u"
> +pci_nvme_err_invalid_opc(uint8_t opc) "invalid opcode 0x%"PRIx8""
> +pci_nvme_err_invalid_admin_opc(uint8_t opc) "invalid admin opcode 0x%"PRIx8""
> +pci_nvme_err_invalid_lba_range(uint64_t start, uint64_t len, uint64_t limit) "Invalid LBA start=%"PRIu64" len=%"PRIu64" limit=%"PRIu64""
> +pci_nvme_err_invalid_del_sq(uint16_t qid) "invalid submission queue deletion, sid=%"PRIu16""
> +pci_nvme_err_invalid_create_sq_cqid(uint16_t cqid) "failed creating submission queue, invalid cqid=%"PRIu16""
> +pci_nvme_err_invalid_create_sq_sqid(uint16_t sqid) "failed creating submission queue, invalid sqid=%"PRIu16""
> +pci_nvme_err_invalid_create_sq_size(uint16_t qsize) "failed creating submission queue, invalid qsize=%"PRIu16""
> +pci_nvme_err_invalid_create_sq_addr(uint64_t addr) "failed creating submission queue, addr=0x%"PRIx64""
> +pci_nvme_err_invalid_create_sq_qflags(uint16_t qflags) "failed creating submission queue, qflags=%"PRIu16""
> +pci_nvme_err_invalid_del_cq_cqid(uint16_t cqid) "failed deleting completion queue, cqid=%"PRIu16""
> +pci_nvme_err_invalid_del_cq_notempty(uint16_t cqid) "failed deleting completion queue, it is not empty, cqid=%"PRIu16""
> +pci_nvme_err_invalid_create_cq_cqid(uint16_t cqid) "failed creating completion queue, cqid=%"PRIu16""
> +pci_nvme_err_invalid_create_cq_size(uint16_t size) "failed creating completion queue, size=%"PRIu16""
> +pci_nvme_err_invalid_create_cq_addr(uint64_t addr) "failed creating completion queue, addr=0x%"PRIx64""
> +pci_nvme_err_invalid_create_cq_vector(uint16_t vector) "failed creating completion queue, vector=%"PRIu16""
> +pci_nvme_err_invalid_create_cq_qflags(uint16_t qflags) "failed creating completion queue, qflags=%"PRIu16""
> +pci_nvme_err_invalid_identify_cns(uint16_t cns) "identify, invalid cns=0x%"PRIx16""
> +pci_nvme_err_invalid_getfeat(int dw10) "invalid get features, dw10=0x%"PRIx32""
> +pci_nvme_err_invalid_setfeat(uint32_t dw10) "invalid set features, dw10=0x%"PRIx32""
> +pci_nvme_err_startfail_cq(void) "nvme_start_ctrl failed because there are non-admin completion queues"
> +pci_nvme_err_startfail_sq(void) "nvme_start_ctrl failed because there are non-admin submission queues"
> +pci_nvme_err_startfail_nbarasq(void) "nvme_start_ctrl failed because the admin submission queue address is null"
> +pci_nvme_err_startfail_nbaracq(void) "nvme_start_ctrl failed because the admin completion queue address is null"
> +pci_nvme_err_startfail_asq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin submission queue address is misaligned: 0x%"PRIx64""
> +pci_nvme_err_startfail_acq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin completion queue address is misaligned: 0x%"PRIx64""
> +pci_nvme_err_startfail_page_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too small: log2size=%u, min=%u"
> +pci_nvme_err_startfail_page_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too large: log2size=%u, max=%u"
> +pci_nvme_err_startfail_cqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too small: log2size=%u, min=%u"
> +pci_nvme_err_startfail_cqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too large: log2size=%u, max=%u"
> +pci_nvme_err_startfail_sqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too small: log2size=%u, min=%u"
> +pci_nvme_err_startfail_sqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too large: log2size=%u, max=%u"
> +pci_nvme_err_startfail_asqent_sz_zero(void) "nvme_start_ctrl failed because the admin submission queue size is zero"
> +pci_nvme_err_startfail_acqent_sz_zero(void) "nvme_start_ctrl failed because the admin completion queue size is zero"
> +pci_nvme_err_startfail(void) "setting controller enable bit failed"
>
> # Traces for undefined behavior
> -nvme_ub_mmiowr_misaligned32(uint64_t offset) "MMIO write not 32-bit aligned, offset=0x%"PRIx64""
> -nvme_ub_mmiowr_toosmall(uint64_t offset, unsigned size) "MMIO write smaller than 32 bits, offset=0x%"PRIx64", size=%u"
> -nvme_ub_mmiowr_intmask_with_msix(void) "undefined access to interrupt mask set when MSI-X is enabled"
> -nvme_ub_mmiowr_ro_csts(void) "attempted to set a read only bit of controller status"
> -nvme_ub_mmiowr_ssreset_w1c_unsupported(void) "attempted to W1C CSTS.NSSRO but CAP.NSSRS is zero (not supported)"
> -nvme_ub_mmiowr_ssreset_unsupported(void) "attempted NVM subsystem reset but CAP.NSSRS is zero (not supported)"
> -nvme_ub_mmiowr_cmbloc_reserved(void) "invalid write to reserved CMBLOC when CMBSZ is zero, ignored"
> -nvme_ub_mmiowr_cmbsz_readonly(void) "invalid write to read only CMBSZ, ignored"
> -nvme_ub_mmiowr_invalid(uint64_t offset, uint64_t data) "invalid MMIO write, offset=0x%"PRIx64", data=0x%"PRIx64""
> -nvme_ub_mmiord_misaligned32(uint64_t offset) "MMIO read not 32-bit aligned, offset=0x%"PRIx64""
> -nvme_ub_mmiord_toosmall(uint64_t offset) "MMIO read smaller than 32-bits, offset=0x%"PRIx64""
> -nvme_ub_mmiord_invalid_ofs(uint64_t offset) "MMIO read beyond last register, offset=0x%"PRIx64", returning 0"
> -nvme_ub_db_wr_misaligned(uint64_t offset) "doorbell write not 32-bit aligned, offset=0x%"PRIx64", ignoring"
> -nvme_ub_db_wr_invalid_cq(uint32_t qid) "completion queue doorbell write for nonexistent queue, cqid=%"PRIu32", ignoring"
> -nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t new_head) "completion queue doorbell write value beyond queue size, cqid=%"PRIu32", new_head=%"PRIu16", ignoring"
> -nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write for nonexistent queue, sqid=%"PRIu32", ignoring"
> -nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail) "submission queue doorbell write value beyond queue size, sqid=%"PRIu32", new_head=%"PRIu16", ignoring"
> +pci_nvme_ub_mmiowr_misaligned32(uint64_t offset) "MMIO write not 32-bit aligned, offset=0x%"PRIx64""
> +pci_nvme_ub_mmiowr_toosmall(uint64_t offset, unsigned size) "MMIO write smaller than 32 bits, offset=0x%"PRIx64", size=%u"
> +pci_nvme_ub_mmiowr_intmask_with_msix(void) "undefined access to interrupt mask set when MSI-X is enabled"
> +pci_nvme_ub_mmiowr_ro_csts(void) "attempted to set a read only bit of controller status"
> +pci_nvme_ub_mmiowr_ssreset_w1c_unsupported(void) "attempted to W1C CSTS.NSSRO but CAP.NSSRS is zero (not supported)"
> +pci_nvme_ub_mmiowr_ssreset_unsupported(void) "attempted NVM subsystem reset but CAP.NSSRS is zero (not supported)"
> +pci_nvme_ub_mmiowr_cmbloc_reserved(void) "invalid write to reserved CMBLOC when CMBSZ is zero, ignored"
> +pci_nvme_ub_mmiowr_cmbsz_readonly(void) "invalid write to read only CMBSZ, ignored"
> +pci_nvme_ub_mmiowr_invalid(uint64_t offset, uint64_t data) "invalid MMIO write, offset=0x%"PRIx64", data=0x%"PRIx64""
> +pci_nvme_ub_mmiord_misaligned32(uint64_t offset) "MMIO read not 32-bit aligned, offset=0x%"PRIx64""
> +pci_nvme_ub_mmiord_toosmall(uint64_t offset) "MMIO read smaller than 32-bits, offset=0x%"PRIx64""
> +pci_nvme_ub_mmiord_invalid_ofs(uint64_t offset) "MMIO read beyond last register, offset=0x%"PRIx64", returning 0"
> +pci_nvme_ub_db_wr_misaligned(uint64_t offset) "doorbell write not 32-bit aligned, offset=0x%"PRIx64", ignoring"
> +pci_nvme_ub_db_wr_invalid_cq(uint32_t qid) "completion queue doorbell write for nonexistent queue, cqid=%"PRIu32", ignoring"
> +pci_nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t new_head) "completion queue doorbell write value beyond queue size, cqid=%"PRIu32", new_head=%"PRIu16", ignoring"
> +pci_nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write for nonexistent queue, sqid=%"PRIu32", ignoring"
> +pci_nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail) "submission queue doorbell write value beyond queue size, sqid=%"PRIu32", new_head=%"PRIu16", ignoring"
>
> # xen-block.c
> xen_block_realize(const char *type, uint32_t disk, uint32_t partition) "%s d%up%u"
I only did a cursory look a this as opposed to applying the same rename with a script and checking that it is the same,
(I can't seem to find the script anymore)
thus I might have missed something.
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 06/16] nvme: refactor nvme_addr_read
2020-04-15 13:01 ` [PATCH v2 06/16] nvme: refactor nvme_addr_read Klaus Jensen
@ 2020-04-21 10:35 ` Maxim Levitsky
0 siblings, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 10:35 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Pull the controller memory buffer check to its own function. The check
> will be used on its own in later patches.
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Acked-by: Keith Busch <kbusch@kernel.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/nvme.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 563cccae6795..d026985f62d0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -55,14 +55,22 @@
>
> static void nvme_process_sq(void *opaque);
>
> +static bool nvme_addr_is_cmb(NvmeCtrl *n, hwaddr addr)
> +{
> + hwaddr low = n->ctrl_mem.addr;
> + hwaddr hi = n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size);
> +
> + return addr >= low && addr < hi;
> +}
> +
> static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
> {
> - if (n->cmbsz && addr >= n->ctrl_mem.addr &&
> - addr < (n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size))) {
> + if (n->cmbsz && nvme_addr_is_cmb(n, addr)) {
> memcpy(buf, (void *)&n->cmbuf[addr - n->ctrl_mem.addr], size);
> - } else {
> - pci_dma_read(&n->parent_obj, addr, buf, size);
> + return;
> }
> +
> + pci_dma_read(&n->parent_obj, addr, buf, size);
> }
>
> static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
Looks like I forgot to add reviewed-by to the patch from V6,
but it looks all right after our discussion so,
Reviewed-by: Maxim Levitsky <mlevitsky@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 08/16] nvme: remove redundant cmbloc/cmbsz members
2020-04-15 13:01 ` [PATCH v2 08/16] nvme: remove redundant cmbloc/cmbsz members Klaus Jensen
@ 2020-04-21 12:05 ` Maxim Levitsky
0 siblings, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 12:05 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/nvme.c | 7 ++-----
> hw/block/nvme.h | 2 --
> 2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 8092c1b46eb1..44856e873fd1 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -66,7 +66,7 @@ static bool nvme_addr_is_cmb(NvmeCtrl *n, hwaddr addr)
>
> static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
> {
> - if (n->cmbsz && nvme_addr_is_cmb(n, addr)) {
> + if (n->bar.cmbsz && nvme_addr_is_cmb(n, addr)) {
> memcpy(buf, (void *)&n->cmbuf[addr - n->ctrl_mem.addr], size);
> return;
> }
> @@ -160,7 +160,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
> if (unlikely(!prp1)) {
> trace_pci_nvme_err_invalid_prp();
> return NVME_INVALID_FIELD | NVME_DNR;
> - } else if (n->cmbsz && prp1 >= n->ctrl_mem.addr &&
> + } else if (n->bar.cmbsz && prp1 >= n->ctrl_mem.addr &&
> prp1 < n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size)) {
> qsg->nsg = 0;
> qemu_iovec_init(iov, num_prps);
> @@ -1425,9 +1425,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
> NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
>
> - n->cmbloc = n->bar.cmbloc;
> - n->cmbsz = n->bar.cmbsz;
> -
> n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
> memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
> "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 1617787af2e9..7eecfd3a50f6 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -88,8 +88,6 @@ typedef struct NvmeCtrl {
> uint32_t num_namespaces;
> uint32_t max_q_ents;
> uint64_t ns_size;
> - uint32_t cmbsz;
> - uint32_t cmbloc;
> uint8_t *cmbuf;
> uint64_t irq_status;
> uint64_t host_timestamp; /* Timestamp sent by the host */
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 09/16] nvme: factor out property/constraint checks
2020-04-15 13:01 ` [PATCH v2 09/16] nvme: factor out property/constraint checks Klaus Jensen
2020-04-15 13:08 ` Philippe Mathieu-Daudé
@ 2020-04-21 14:53 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 14:53 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 43 ++++++++++++++++++++++++++++---------------
> 1 file changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 44856e873fd1..5f9ebbd6a1d5 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1311,24 +1311,19 @@ static const MemoryRegionOps nvme_cmb_ops = {
> },
> };
>
> -static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> +static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
> {
> - NvmeCtrl *n = NVME(pci_dev);
> - NvmeIdCtrl *id = &n->id_ctrl;
> + NvmeParams *params = &n->params;
>
> - int i;
> - int64_t bs_size;
> - uint8_t *pci_conf;
> -
> - if (n->params.num_queues) {
> + if (params->num_queues) {
> warn_report("num_queues is deprecated; please use max_ioqpairs "
> "instead");
>
> - n->params.max_ioqpairs = n->params.num_queues - 1;
> + params->max_ioqpairs = params->num_queues - 1;
> }
>
> - if (n->params.max_ioqpairs < 1 ||
> - n->params.max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
> + if (params->max_ioqpairs < 1 ||
> + params->max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
> error_setg(errp, "max_ioqpairs must be between 1 and %d",
> PCI_MSIX_FLAGS_QSIZE);
> return;
> @@ -1339,16 +1334,34 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> return;
> }
>
> + if (!params->serial) {
> + error_setg(errp, "serial property not set");
> + return;
> + }
> +}
> +
> +static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> +{
> + NvmeCtrl *n = NVME(pci_dev);
> + NvmeIdCtrl *id = &n->id_ctrl;
> + Error *err = NULL;
Yep, lets name it indeed local_err.
> +
> + int i;
> + int64_t bs_size;
> + uint8_t *pci_conf;
> +
> + nvme_check_constraints(n, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> bs_size = blk_getlength(n->conf.blk);
> if (bs_size < 0) {
> error_setg(errp, "could not get backing file size");
> return;
> }
>
> - if (!n->params.serial) {
> - error_setg(errp, "serial property not set");
> - return;
> - }
> blkconf_blocksizes(&n->conf);
> if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
> false, errp)) {
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 10/16] nvme: factor out device state setup
2020-04-15 13:01 ` [PATCH v2 10/16] nvme: factor out device state setup Klaus Jensen
@ 2020-04-21 14:55 ` Maxim Levitsky
0 siblings, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 14:55 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/nvme.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 5f9ebbd6a1d5..45a352b63d89 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1340,6 +1340,17 @@ static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
> }
> }
>
> +static void nvme_init_state(NvmeCtrl *n)
> +{
> + n->num_namespaces = 1;
> + /* add one to max_ioqpairs to account for the admin queue pair */
> + n->reg_size = pow2ceil(NVME_REG_SIZE +
> + 2 * (n->params.max_ioqpairs + 1) * NVME_DB_SIZE);
> + n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
> + n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
> + n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
> +}
> +
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> {
> NvmeCtrl *n = NVME(pci_dev);
> @@ -1356,6 +1367,8 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> return;
> }
>
> + nvme_init_state(n);
> +
> bs_size = blk_getlength(n->conf.blk);
> if (bs_size < 0) {
> error_setg(errp, "could not get backing file size");
> @@ -1374,17 +1387,8 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
> pcie_endpoint_cap_init(pci_dev, 0x80);
>
> - n->num_namespaces = 1;
> -
> - /* add one to max_ioqpairs to account for the admin queue pair */
> - n->reg_size = pow2ceil(NVME_REG_SIZE +
> - 2 * (n->params.max_ioqpairs + 1) * NVME_DB_SIZE);
> n->ns_size = bs_size / (uint64_t)n->num_namespaces;
>
> - n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
> - n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
> - n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
> -
> memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
> "nvme", n->reg_size);
> pci_register_bar(pci_dev, 0,
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 11/16] nvme: factor out block backend setup
2020-04-15 13:01 ` [PATCH v2 11/16] nvme: factor out block backend setup Klaus Jensen
2020-04-15 13:08 ` Philippe Mathieu-Daudé
@ 2020-04-21 15:04 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 15:04 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 45a352b63d89..80da0825d295 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1351,6 +1351,13 @@ static void nvme_init_state(NvmeCtrl *n)
> n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
> }
>
> +static void nvme_init_blk(NvmeCtrl *n, Error **errp)
> +{
> + blkconf_blocksizes(&n->conf);
> + blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
> + false, errp);
> +}
> +
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> {
> NvmeCtrl *n = NVME(pci_dev);
> @@ -1375,9 +1382,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> return;
> }
>
> - blkconf_blocksizes(&n->conf);
> - if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
> - false, errp)) {
> + nvme_init_blk(n, &err);
> + if (err) {
> + error_propagate(errp, err);
> return;
> }
>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 12/16] nvme: add namespace helpers
2020-04-15 13:01 ` [PATCH v2 12/16] nvme: add namespace helpers Klaus Jensen
2020-04-15 13:09 ` Philippe Mathieu-Daudé
@ 2020-04-21 15:41 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 15:41 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Introduce some small helpers to make the next patches easier on the eye.
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 3 +--
> hw/block/nvme.h | 16 ++++++++++++++++
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 80da0825d295..d5244102252c 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1469,8 +1469,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> id_ns->dps = 0;
> id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
> id_ns->ncap = id_ns->nuse = id_ns->nsze =
> - cpu_to_le64(n->ns_size >>
> - id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds);
> + cpu_to_le64(nvme_ns_nlbas(n, ns));
> }
> }
>
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 7eecfd3a50f6..dd932a9e7ebc 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -67,6 +67,17 @@ typedef struct NvmeNamespace {
> NvmeIdNs id_ns;
> } NvmeNamespace;
>
> +static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
> +{
> + NvmeIdNs *id_ns = &ns->id_ns;
> + return &id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(id_ns->flbas)];
> +}
> +
> +static inline uint8_t nvme_ns_lbads(NvmeNamespace *ns)
> +{
> + return nvme_ns_lbaf(ns)->ds;
> +}
> +
> #define TYPE_NVME "nvme"
> #define NVME(obj) \
> OBJECT_CHECK(NvmeCtrl, (obj), TYPE_NVME)
> @@ -101,4 +112,9 @@ typedef struct NvmeCtrl {
> NvmeIdCtrl id_ctrl;
> } NvmeCtrl;
>
> +static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
> +{
> + return n->ns_size >> nvme_ns_lbads(ns);
> +}
> +
> #endif /* HW_NVME_H */
Nitpick: On second thought, these function names do sound quite cryptic, so maybe
at least for nvme_ns_nlbas pick something more readable? maybe nvme_ns_number_of_lbas
or something.
Or add a comment - comment always fixes these kind of issues.
But that doesn't matter much IMHO, so
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 00/16] nvme: refactoring and cleanups
2020-04-21 6:38 ` Klaus Birkelund Jensen
@ 2020-04-21 15:47 ` Kevin Wolf
0 siblings, 0 replies; 48+ messages in thread
From: Kevin Wolf @ 2020-04-21 15:47 UTC (permalink / raw)
To: Klaus Birkelund Jensen
Cc: Beata Michalska, qemu-block, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
Am 21.04.2020 um 08:38 hat Klaus Birkelund Jensen geschrieben:
> On Apr 21 02:38, Keith Busch wrote:
> > The series looks good to me.
> >
> > Reviewed-by: Keith Busch <kbusch@kernel.org>
>
> Thanks for the review Keith!
>
> Kevin, should I rebase this on block-next? I think it might have some
> conflicts with the PMR patch that went in previously.
The series doesn't apply at the moment anyway, I assume it's because of
the PMR patch. So yes, I would appreciate a rebase.
Kevin
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 13/16] nvme: factor out namespace setup
2020-04-15 13:01 ` [PATCH v2 13/16] nvme: factor out namespace setup Klaus Jensen
2020-04-15 13:16 ` Philippe Mathieu-Daudé
@ 2020-04-21 15:57 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 15:57 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 46 ++++++++++++++++++++++++++--------------------
> 1 file changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index d5244102252c..2b007115c302 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1358,6 +1358,27 @@ static void nvme_init_blk(NvmeCtrl *n, Error **errp)
> false, errp);
> }
>
> +static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
> +{
> + int64_t bs_size;
> + NvmeIdNs *id_ns = &ns->id_ns;
> +
> + bs_size = blk_getlength(n->conf.blk);
> + if (bs_size < 0) {
> + error_setg_errno(errp, -bs_size, "could not get backing file size");
> + return;
> + }
> +
> + n->ns_size = bs_size;
> +
> + id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
> + id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(n, ns));
> +
> + /* no thin provisioning */
> + id_ns->ncap = id_ns->nsze;
> + id_ns->nuse = id_ns->ncap;
> +}
> +
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> {
> NvmeCtrl *n = NVME(pci_dev);
> @@ -1365,7 +1386,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> Error *err = NULL;
>
> int i;
> - int64_t bs_size;
> uint8_t *pci_conf;
>
> nvme_check_constraints(n, &err);
> @@ -1376,12 +1396,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>
> nvme_init_state(n);
>
> - bs_size = blk_getlength(n->conf.blk);
> - if (bs_size < 0) {
> - error_setg(errp, "could not get backing file size");
> - return;
> - }
> -
> nvme_init_blk(n, &err);
> if (err) {
> error_propagate(errp, err);
> @@ -1394,8 +1408,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
> pcie_endpoint_cap_init(pci_dev, 0x80);
>
> - n->ns_size = bs_size / (uint64_t)n->num_namespaces;
> -
> memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
> "nvme", n->reg_size);
> pci_register_bar(pci_dev, 0,
> @@ -1459,17 +1471,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> }
>
> for (i = 0; i < n->num_namespaces; i++) {
> - NvmeNamespace *ns = &n->namespaces[i];
> - NvmeIdNs *id_ns = &ns->id_ns;
> - id_ns->nsfeat = 0;
> - id_ns->nlbaf = 0;
> - id_ns->flbas = 0;
> - id_ns->mc = 0;
> - id_ns->dpc = 0;
> - id_ns->dps = 0;
> - id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
> - id_ns->ncap = id_ns->nuse = id_ns->nsze =
> - cpu_to_le64(nvme_ns_nlbas(n, ns));
> + nvme_init_namespace(n, &n->namespaces[i], &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> }
> }
>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 14/16] nvme: factor out pci setup
2020-04-15 13:01 ` [PATCH v2 14/16] nvme: factor out pci setup Klaus Jensen
2020-04-15 13:14 ` Philippe Mathieu-Daudé
@ 2020-04-21 15:59 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 15:59 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 30 ++++++++++++++++++------------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 2b007115c302..906ae595025a 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1379,6 +1379,22 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
> id_ns->nuse = id_ns->ncap;
> }
>
> +static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> +{
> + uint8_t *pci_conf = pci_dev->config;
> +
> + pci_conf[PCI_INTERRUPT_PIN] = 1;
> + pci_config_set_prog_interface(pci_conf, 0x2);
> + pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
> + pcie_endpoint_cap_init(pci_dev, 0x80);
> +
> + memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n, "nvme",
> + n->reg_size);
> + pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
> + PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem);
> + msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
> +}
> +
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> {
> NvmeCtrl *n = NVME(pci_dev);
> @@ -1402,19 +1418,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> return;
> }
>
> + nvme_init_pci(n, pci_dev);
> +
> pci_conf = pci_dev->config;
> - pci_conf[PCI_INTERRUPT_PIN] = 1;
> - pci_config_set_prog_interface(pci_dev->config, 0x2);
> - pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
> - pcie_endpoint_cap_init(pci_dev, 0x80);
> -
> - memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
> - "nvme", n->reg_size);
> - pci_register_bar(pci_dev, 0,
> - PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
> - &n->iomem);
> - msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
> -
> id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
> id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
> strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 15/16] nvme: factor out cmb setup
2020-04-15 13:01 ` [PATCH v2 15/16] nvme: factor out cmb setup Klaus Jensen
@ 2020-04-21 16:10 ` Maxim Levitsky
0 siblings, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 16:10 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/nvme.c | 49 +++++++++++++++++++++++++++----------------------
> 1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 906ae595025a..4c28d75e0fc8 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -46,6 +46,7 @@
>
> #define NVME_REG_SIZE 0x1000
> #define NVME_DB_SIZE 4
> +#define NVME_CMB_BIR 2
>
> #define NVME_GUEST_ERR(trace, fmt, ...) \
> do { \
> @@ -1379,6 +1380,28 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
> id_ns->nuse = id_ns->ncap;
> }
>
> +static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
> +{
> + NVME_CMBLOC_SET_BIR(n->bar.cmbloc, NVME_CMB_BIR);
> + NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
> +
> + NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
> + NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
> + NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
> + NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
> + NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
> + NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
> + NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
> +
> + n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
> + memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
> + "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
> + pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
> + PCI_BASE_ADDRESS_SPACE_MEMORY |
> + PCI_BASE_ADDRESS_MEM_TYPE_64 |
> + PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
> +}
> +
> static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> {
> uint8_t *pci_conf = pci_dev->config;
> @@ -1393,6 +1416,10 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
> PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem);
> msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
> +
> + if (n->params.cmb_size_mb) {
> + nvme_init_cmb(n, pci_dev);
> + }
> }
>
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> @@ -1454,28 +1481,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> n->bar.vs = 0x00010200;
> n->bar.intmc = n->bar.intms = 0;
>
> - if (n->params.cmb_size_mb) {
> -
> - NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2);
> - NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
> -
> - NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
> - NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
> - NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
> - NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
> - NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
> - NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
> - NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
> -
> - n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
> - memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
> - "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
> - pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
> - PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |
> - PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
> -
> - }
> -
> for (i = 0; i < n->num_namespaces; i++) {
> nvme_init_namespace(n, &n->namespaces[i], &err);
> if (err) {
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 16/16] nvme: factor out controller identify setup
2020-04-15 13:01 ` [PATCH v2 16/16] nvme: factor out controller identify setup Klaus Jensen
2020-04-15 13:06 ` Philippe Mathieu-Daudé
@ 2020-04-21 16:17 ` Maxim Levitsky
1 sibling, 0 replies; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 16:17 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/nvme.c | 52 +++++++++++++++++++++++++++----------------------
> 1 file changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 4c28d75e0fc8..804f24719dce 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1422,32 +1422,11 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
> }
> }
>
> -static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> +static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> {
> - NvmeCtrl *n = NVME(pci_dev);
> NvmeIdCtrl *id = &n->id_ctrl;
> - Error *err = NULL;
> + uint8_t *pci_conf = pci_dev->config;
>
> - int i;
> - uint8_t *pci_conf;
> -
> - nvme_check_constraints(n, &err);
> - if (err) {
> - error_propagate(errp, err);
> - return;
> - }
> -
> - nvme_init_state(n);
> -
> - nvme_init_blk(n, &err);
> - if (err) {
> - error_propagate(errp, err);
> - return;
> - }
> -
> - nvme_init_pci(n, pci_dev);
> -
> - pci_conf = pci_dev->config;
> id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
> id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
> strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
> @@ -1481,6 +1460,33 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> n->bar.vs = 0x00010200;
> n->bar.intmc = n->bar.intms = 0;
>
> +
> +}
> +
> +static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> +{
> + NvmeCtrl *n = NVME(pci_dev);
> + Error *err = NULL;
> +
> + int i;
> +
> + nvme_check_constraints(n, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> +
> + nvme_init_state(n);
> + nvme_init_blk(n, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + nvme_init_pci(n, pci_dev);
> + nvme_init_ctrl(n, pci_dev);
> +
> for (i = 0; i < n->num_namespaces; i++) {
> nvme_init_namespace(n, &n->namespaces[i], &err);
> if (err) {
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 00/16] nvme: refactoring and cleanups
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
` (18 preceding siblings ...)
2020-04-20 17:38 ` Keith Busch
@ 2020-04-21 16:24 ` Maxim Levitsky
2020-04-22 6:19 ` Klaus Birkelund Jensen
19 siblings, 1 reply; 48+ messages in thread
From: Maxim Levitsky @ 2020-04-21 16:24 UTC (permalink / raw)
To: Klaus Jensen, qemu-block
Cc: Kevin Wolf, Beata Michalska, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
On Wed, 2020-04-15 at 15:01 +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Changes since v1
> ~~~~~~~~~~~~~~~~
> * nvme: fix pci doorbell size calculation
> - added some defines and a better comment (Philippe)
>
> * nvme: rename trace events to pci_nvme
> - changed the prefix from nvme_dev to pci_nvme (Philippe)
>
> * nvme: add max_ioqpairs device parameter
> - added a deprecation comment. I doubt this will go in until 5.1, so
> changed it to "deprecated from 5.1" (Philippe)
>
> * nvme: factor out property/constraint checks
> * nvme: factor out block backend setup
> - changed to return void and propagate errors in proper QEMU style
> (Philippe)
>
> * nvme: add namespace helpers
> - use the helper immediately (Philippe)
>
> * nvme: factor out pci setup
> - removed setting of vendor and device id which is already inherited
> from nvme_class_init() (Philippe)
>
> * nvme: factor out cmb setup
> - add lost comment (Philippe)
>
>
> Klaus Jensen (16):
> nvme: fix pci doorbell size calculation
> nvme: rename trace events to pci_nvme
> nvme: remove superfluous breaks
> nvme: move device parameters to separate struct
> nvme: use constants in identify
> nvme: refactor nvme_addr_read
> nvme: add max_ioqpairs device parameter
> nvme: remove redundant cmbloc/cmbsz members
> nvme: factor out property/constraint checks
> nvme: factor out device state setup
> nvme: factor out block backend setup
> nvme: add namespace helpers
> nvme: factor out namespace setup
> nvme: factor out pci setup
> nvme: factor out cmb setup
> nvme: factor out controller identify setup
>
> hw/block/nvme.c | 433 ++++++++++++++++++++++++------------------
> hw/block/nvme.h | 36 +++-
> hw/block/trace-events | 172 ++++++++---------
> include/block/nvme.h | 8 +
> 4 files changed, 372 insertions(+), 277 deletions(-)
>
Should I also review the V7 series or I should wait for V8 which will not include these cleanups?
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 00/16] nvme: refactoring and cleanups
2020-04-21 16:24 ` Maxim Levitsky
@ 2020-04-22 6:19 ` Klaus Birkelund Jensen
0 siblings, 0 replies; 48+ messages in thread
From: Klaus Birkelund Jensen @ 2020-04-22 6:19 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, Beata Michalska, qemu-block, Klaus Jensen,
qemu-devel, Max Reitz, Keith Busch, Javier Gonzalez,
Philippe Mathieu-Daudé
On Apr 21 19:24, Maxim Levitsky wrote:
> Should I also review the V7 series or I should wait for V8 which will
> not include these cleanups?
Hi Maxim,
Just wait for another series - I don't think I will post a v8, I will
chop op the series into smaller ones instead.
Most patches will hopefully not change too much, so should keep your
Reviewed-by's ;)
Thanks,
Klaus
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 13/16] nvme: factor out namespace setup
2020-04-16 6:03 ` Klaus Birkelund Jensen
@ 2020-04-24 10:13 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-24 10:13 UTC (permalink / raw)
To: Klaus Birkelund Jensen
Cc: Kevin Wolf, Beata Michalska, qemu-block, Klaus Jensen,
qemu-devel, Max Reitz, Keith Busch, Javier Gonzalez,
Maxim Levitsky
On 4/16/20 8:03 AM, Klaus Birkelund Jensen wrote:
> On Apr 15 15:26, Philippe Mathieu-Daudé wrote:
>> On 4/15/20 3:20 PM, Klaus Birkelund Jensen wrote:
>>>
>>> I'll get the v1.3 series ready next.
>>>
>>
>> Cool. What really matters (to me) is seeing tests. If we can merge tests
>> (without multiple namespaces) before the rest of your series, even better.
>> Tests give reviewers/maintainers confidence that code isn't breaking ;)
>>
>
> The patches that I contribute have been pretty extensively tested by
> various means in a "host setting" (e.g. blktests and some internal
> tools), which really exercise the device by doing heavy I/O, testing for
> compliance and also just being mean to it (e.g. tripping bus mastering
> while doing I/O).
>
> Don't misunderstand me as trying to weasel my way out of writing tests,
> but I just want to understand the scope of the tests that you are
> looking for? I believe (hope!) that you are not asking me to implement a
> user-space NVMe driver in the test, so I assume the tests should varify
> more low level details?
I was thinking about something rather simple.
So you are adding the "multiple namespaces" feature, we want to test it.
If you can demonstrate it works with few I/O calls you could try with
qtest, such:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg695421.html
If it requires complex commands, since the user-space tools already
exist, you can use an acceptance test booting Linux, installing the NVMe
tools and use them. See tests/acceptance/linux_ssh_mips_malta.py or
https://www.mail-archive.com/qemu-devel@nongnu.org/msg656319.html
Regards,
Phil.
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2020-04-24 10:15 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-15 13:01 [PATCH v2 00/16] nvme: refactoring and cleanups Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 01/16] nvme: fix pci doorbell size calculation Klaus Jensen
2020-04-15 13:13 ` Philippe Mathieu-Daudé
2020-04-21 9:39 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 02/16] nvme: rename trace events to pci_nvme Klaus Jensen
2020-04-15 13:04 ` Philippe Mathieu-Daudé
2020-04-21 9:43 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 03/16] nvme: remove superfluous breaks Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 04/16] nvme: move device parameters to separate struct Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 05/16] nvme: use constants in identify Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 06/16] nvme: refactor nvme_addr_read Klaus Jensen
2020-04-21 10:35 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 07/16] nvme: add max_ioqpairs device parameter Klaus Jensen
2020-04-15 13:01 ` [PATCH v2 08/16] nvme: remove redundant cmbloc/cmbsz members Klaus Jensen
2020-04-21 12:05 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 09/16] nvme: factor out property/constraint checks Klaus Jensen
2020-04-15 13:08 ` Philippe Mathieu-Daudé
2020-04-21 14:53 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 10/16] nvme: factor out device state setup Klaus Jensen
2020-04-21 14:55 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 11/16] nvme: factor out block backend setup Klaus Jensen
2020-04-15 13:08 ` Philippe Mathieu-Daudé
2020-04-21 15:04 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 12/16] nvme: add namespace helpers Klaus Jensen
2020-04-15 13:09 ` Philippe Mathieu-Daudé
2020-04-21 15:41 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 13/16] nvme: factor out namespace setup Klaus Jensen
2020-04-15 13:16 ` Philippe Mathieu-Daudé
2020-04-15 13:20 ` Klaus Birkelund Jensen
2020-04-15 13:26 ` Philippe Mathieu-Daudé
2020-04-16 6:03 ` Klaus Birkelund Jensen
2020-04-24 10:13 ` Philippe Mathieu-Daudé
2020-04-21 15:57 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 14/16] nvme: factor out pci setup Klaus Jensen
2020-04-15 13:14 ` Philippe Mathieu-Daudé
2020-04-21 15:59 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 15/16] nvme: factor out cmb setup Klaus Jensen
2020-04-21 16:10 ` Maxim Levitsky
2020-04-15 13:01 ` [PATCH v2 16/16] nvme: factor out controller identify setup Klaus Jensen
2020-04-15 13:06 ` Philippe Mathieu-Daudé
2020-04-21 16:17 ` Maxim Levitsky
2020-04-15 14:29 ` [PATCH v2 00/16] nvme: refactoring and cleanups no-reply
2020-04-20 5:14 ` Klaus Birkelund Jensen
2020-04-20 17:38 ` Keith Busch
2020-04-21 6:38 ` Klaus Birkelund Jensen
2020-04-21 15:47 ` Kevin Wolf
2020-04-21 16:24 ` Maxim Levitsky
2020-04-22 6:19 ` Klaus Birkelund Jensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.