* [RFC 0/1] hw/nvme: add atomic write support
@ 2024-04-15 23:46 Alan Adamson
2024-04-15 23:46 ` [RFC 1/1] " Alan Adamson
2024-05-08 10:17 ` [RFC 0/1] " Klaus Jensen
0 siblings, 2 replies; 3+ messages in thread
From: Alan Adamson @ 2024-04-15 23:46 UTC (permalink / raw
To: qemu-devel; +Cc: alan.adamson, kbusch, its, qemu-block
Since there is discussion in the Linux NVMe Driver community to add NVMe Atomic Write
support, it would be desirable to test it with qemu nvme emulation.
Initially, this RFC will focus on supporting NVMe controller atomic write parameters(AWUN,
AWUPF, and ACWU) but will be extended to support Namespace parameters (NAWUN, NAWUPF
and NACWU).
Atomic Write Parameters for NVMe QEMU
-------------------------------------
New NVMe QEMU Parameters (See NVMe Specification for details):
atomic.dn (default off) - Set the value of Disable Normal.
atomic.awun=UINT16 (default: 0)
atomic.awupf=UINT16 (default: 0)
atomic.acwu=UINT16 (default: 0)
qemu command line example:
qemu-system-x86_64 -cpu host --enable-kvm -smp cpus=4 -no-reboot -m 8192M -drive file=./disk.img,if=ide \
-boot c -device e1000,netdev=net0,mac=DE:CC:CC:EF:99:88 -netdev tap,id=net0 \
-device nvme,id=nvme-ctrl-0,serial=nvme-1,atomic.dn=off,atomic.awun=63,atomic.awupf=63,atomic.acwu=0 \
-drive file=./nvme.img,if=none,id=nvm-1 -device nvme-ns,drive=nvm-1,bus=nvme-ctrl-0 nvme-ns,drive=nvm-1,bus=nvme-ctrl-0
Making Writes Atomic:
---------------------
- Prior to a command being pulled off the SQ and executed, a check is made to see if it
conflicts "atomically" with a currently executing command.
- All currently executing commands on the same namespace, across all SQs need to be checked.
- If an atomic conflict is detected, the command is not started and remains on the queue.
Testing
-------
NVMe QEMU Parameters used: atomic.dn=off,atomic.awun=63,atomic.awupf=63,atomic.acwu=0
# nvme id-ctrl /dev/nvme0 | grep awun
awun : 63
# nvme id-ctrl /dev/nvme0 | grep awupf
awupf : 63
# nvme id-ctrl /dev/nvme0 | grep acwu
acwu : 0 < Since qemu-nvme doesn't support Compare and Write, this is always zero
# nvme get-feature /dev/nvme0 -f 0xa
get-feature:0x0a (Write Atomicity Normal), Current value:00000000
#
# fio --filename=/dev/nvme0n1 --direct=1 --rw=randwrite --bs=32k --iodepth=256 --name=iops --numjobs=50 --verify=crc64 --verify_fatal=1 --ioengine=libaio
When executed without atomic write support, eventually the following error will be
observed:
crc64: verify failed at file /dev/nvme0n1 offset 857669632, length 32768
(requested block: offset=857669632, length=32768, flags=88)
Expected CRC: 9c87d3539dafdca0
Received CRC: d521f7ea3b69d2ee
When executed with atomic write support, this error no longer happens.
Questions
---------
AWUN vs AWUPF - Does the nvme emulation need to do treat these differently? Currently the
larger of the two will be used as the max atomic write size.
Future Work
-----------
- Namespace support (NAWUN, NAWUPF and NACWU)
- Namespace Boundary support (NABSN, NABO, and NABSPF)
- Atomic Compare and Write Unit (ACWU)
Alan Adamson (1):
nvme: add atomic write support
hw/nvme/ctrl.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++-
hw/nvme/nvme.h | 17 ++++++
2 files changed, 163 insertions(+), 1 deletion(-)
--
2.39.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* [RFC 1/1] hw/nvme: add atomic write support
2024-04-15 23:46 [RFC 0/1] hw/nvme: add atomic write support Alan Adamson
@ 2024-04-15 23:46 ` Alan Adamson
2024-05-08 10:17 ` [RFC 0/1] " Klaus Jensen
1 sibling, 0 replies; 3+ messages in thread
From: Alan Adamson @ 2024-04-15 23:46 UTC (permalink / raw
To: qemu-devel; +Cc: alan.adamson, kbusch, its, qemu-block
Forces writes to be atomic based on new atomic write parameters.
New NVMe QEMU Parameters (See NVMe Specification for details):
atomic.dn (default off) - Set the value of Disable Normal.
atomic.awun=UINT16 (default: 0)
atomic.awupf=UINT16 (default: 0)
atomic.acwu=UINT16 (default: 0)
Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
---
hw/nvme/ctrl.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++-
hw/nvme/nvme.h | 17 ++++++
2 files changed, 163 insertions(+), 1 deletion(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 127c3d238346..5d19965122d0 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -40,6 +40,9 @@
* sriov_vi_flexible=<N[optional]> \
* sriov_max_vi_per_vf=<N[optional]> \
* sriov_max_vq_per_vf=<N[optional]> \
+ * atomic.awun<N[optional]>, \
+ * atomic.awupf<N[optional]>, \
+ * atomic.acwu<N[optional]>, \
* subsys=<subsys_id>
* -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
* zoned=<true|false[optional]>, \
@@ -254,6 +257,7 @@ static const uint32_t nvme_feature_cap[NVME_FID_MAX] = {
[NVME_ERROR_RECOVERY] = NVME_FEAT_CAP_CHANGE | NVME_FEAT_CAP_NS,
[NVME_VOLATILE_WRITE_CACHE] = NVME_FEAT_CAP_CHANGE,
[NVME_NUMBER_OF_QUEUES] = NVME_FEAT_CAP_CHANGE,
+ [NVME_WRITE_ATOMICITY] = NVME_FEAT_CAP_CHANGE,
[NVME_ASYNCHRONOUS_EVENT_CONF] = NVME_FEAT_CAP_CHANGE,
[NVME_TIMESTAMP] = NVME_FEAT_CAP_CHANGE,
[NVME_HOST_BEHAVIOR_SUPPORT] = NVME_FEAT_CAP_CHANGE,
@@ -6071,6 +6075,9 @@ defaults:
}
goto out;
+ break;
+ case NVME_WRITE_ATOMICITY:
+ result = n->dn;
break;
default:
result = nvme_feature_default[fid];
@@ -6154,6 +6161,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
uint8_t save = NVME_SETFEAT_SAVE(dw10);
uint16_t status;
int i;
+ NvmeIdCtrl *id = &n->id_ctrl;
+ NvmeAtomic *atomic = &n->atomic;
trace_pci_nvme_setfeat(nvme_cid(req), nsid, fid, save, dw11);
@@ -6306,6 +6315,21 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
return NVME_CMD_SEQ_ERROR | NVME_DNR;
case NVME_FDP_EVENTS:
return nvme_set_feature_fdp_events(n, ns, req);
+ case NVME_WRITE_ATOMICITY:
+
+ /*
+ * Since NAWUN and NAWUPF are not currently supported, this
+ * has no affect.
+ */
+ n->dn = 0x1 & dw11;
+
+ if (n->dn) {
+ atomic->atomic_max_write_size = id->awupf + 1;
+ } else {
+ atomic->atomic_max_write_size = id->awun + 1;
+ }
+
+ break;
default:
return NVME_FEAT_NOT_CHANGEABLE | NVME_DNR;
}
@@ -7003,16 +7027,76 @@ static void nvme_update_sq_tail(NvmeSQueue *sq)
trace_pci_nvme_update_sq_tail(sq->sqid, sq->tail);
}
+#define NVME_ATOMIC_NO_START 0
+#define NVME_ATOMIC_START_ATOMIC 1
+#define NVME_ATOMIC_START_NONATOMIC 2
+
+static int nvme_atomic_write_check(NvmeCtrl *n, NvmeCmd *cmd, NvmeAtomic *atomic)
+{
+ NvmeRwCmd *rw = (NvmeRwCmd *)cmd;
+ uint64_t slba = le64_to_cpu(rw->slba);
+ uint32_t nlb = (uint32_t)le16_to_cpu(rw->nlb);
+ uint64_t elba = slba + nlb;
+ bool cmd_atomic_wr = 1;
+ int i;
+
+ if ((cmd->opcode == NVME_CMD_READ) || ((cmd->opcode == NVME_CMD_WRITE) &&
+ ((rw->nlb + 1) > atomic->atomic_max_write_size))) {
+ cmd_atomic_wr = 0;
+ }
+
+ for (i = 1; i < n->params.max_ioqpairs + 1; i++) {
+ NvmeSQueue *sq;
+ NvmeRequest *r;
+ NvmeRwCmd *req_rw;
+ uint64_t req_slba;
+ uint32_t req_nlb;
+ uint64_t req_elba;
+
+ sq = n->sq[i];
+ if (!sq) {
+ break;
+ }
+
+ QTAILQ_FOREACH(r, &sq->out_req_list, entry) {
+ req_rw = (NvmeRwCmd *)&r->cmd;
+
+ if (cmd->nsid == r->ns->params.nsid) {
+ req_slba = le64_to_cpu(req_rw->slba);
+ req_nlb = (uint32_t)le16_to_cpu(req_rw->nlb);
+ req_elba = req_slba + req_nlb;
+
+ if (cmd_atomic_wr) {
+ if ((elba >= req_slba) && (slba <= req_elba)) {
+ return NVME_ATOMIC_NO_START;
+ }
+ } else {
+ if (r->atomic_write && ((elba >= req_slba) &&
+ (slba <= req_elba))) {
+ return NVME_ATOMIC_NO_START;
+ }
+ }
+ }
+ }
+ }
+ if (cmd_atomic_wr) {
+ return NVME_ATOMIC_START_ATOMIC;
+ }
+ return NVME_ATOMIC_START_NONATOMIC;
+}
+
static void nvme_process_sq(void *opaque)
{
NvmeSQueue *sq = opaque;
NvmeCtrl *n = sq->ctrl;
NvmeCQueue *cq = n->cq[sq->cqid];
-
+ NvmeAtomic *atomic = &n->atomic;
uint16_t status;
hwaddr addr;
NvmeCmd cmd;
NvmeRequest *req;
+ int ret;
+ bool set_atomic = 0;
if (n->dbbuf_enabled) {
nvme_update_sq_tail(sq);
@@ -7026,6 +7110,23 @@ static void nvme_process_sq(void *opaque)
stl_le_p(&n->bar.csts, NVME_CSTS_FAILED);
break;
}
+
+ if (sq->sqid && atomic->atomic_writes) {
+ qemu_mutex_lock(&atomic->atomic_lock);
+ ret = nvme_atomic_write_check(n, &cmd, atomic);
+ switch (ret) {
+ case NVME_ATOMIC_NO_START:
+ qemu_bh_schedule(sq->bh);
+ qemu_mutex_unlock(&atomic->atomic_lock);
+ return;
+ case NVME_ATOMIC_START_ATOMIC:
+ set_atomic = 1;
+ break;
+ case NVME_ATOMIC_START_NONATOMIC:
+ default:
+ break;
+ }
+ }
nvme_inc_sq_head(sq);
req = QTAILQ_FIRST(&sq->req_list);
@@ -7035,6 +7136,11 @@ static void nvme_process_sq(void *opaque)
req->cqe.cid = cmd.cid;
memcpy(&req->cmd, &cmd, sizeof(NvmeCmd));
+ if (atomic->atomic_writes) {
+ req->atomic_write = set_atomic;
+ qemu_mutex_unlock(&atomic->atomic_lock);
+ }
+
status = sq->sqid ? nvme_io_cmd(n, req) :
nvme_admin_cmd(n, req);
if (status != NVME_NO_COMPLETE) {
@@ -7138,6 +7244,8 @@ static void nvme_ctrl_reset(NvmeCtrl *n, NvmeResetType rst)
n->outstanding_aers = 0;
n->qs_created = false;
+ n->dn = n->params.atomic_dn; /* Set Disable Normal */
+
nvme_update_msixcap_ts(pci_dev, n->conf_msix_qsize);
if (pci_is_vf(pci_dev)) {
@@ -8191,6 +8299,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
uint8_t *pci_conf = pci_dev->config;
uint64_t cap = ldq_le_p(&n->bar.cap);
NvmeSecCtrlEntry *sctrl = nvme_sctrl(n);
+ NvmeAtomic *atomic = &n->atomic;
uint32_t ctratt;
id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
@@ -8298,6 +8407,38 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
if (pci_is_vf(pci_dev) && !sctrl->scs) {
stl_le_p(&n->bar.csts, NVME_CSTS_FAILED);
}
+
+ /* Atomic Write */
+ id->awun = n->params.atomic_awun;
+ id->awupf = n->params.atomic_awupf;
+ if (n->params.atomic_acwu) { /* ACWU is currently not supported */
+ printf("Atomic Paramters: ACWU is not supported\n");
+ id->acwu = 0;
+ }
+ n->dn = n->params.atomic_dn;
+
+ if (id->awun || id->awupf) {
+ if (id->awupf > id->awun) {
+ printf(" AWUPF must be <= to AWUN: Setting AWUPF to %d\n",
+ id->awun);
+ id->awupf = id->awun;
+ }
+
+ if (n->dn) {
+ atomic->atomic_max_write_size = id->awupf + 1;
+ } else {
+ atomic->atomic_max_write_size = id->awun + 1;
+ }
+
+ if (atomic->atomic_max_write_size == 1)
+ atomic->atomic_writes = 0;
+ else {
+ qemu_mutex_init(&atomic->atomic_lock);
+ atomic->atomic_writes = 1;
+ }
+ }
+ printf("Atomic Paramters: AWUN=%d AWUPF=%d ACWU=%d DN=%d\n",
+ id->awun, id->awupf, id->acwu, n->dn);
}
static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
@@ -8451,6 +8592,10 @@ static Property nvme_props[] = {
params.sriov_max_vq_per_vf, 0),
DEFINE_PROP_BOOL("msix-exclusive-bar", NvmeCtrl, params.msix_exclusive_bar,
false),
+ DEFINE_PROP_BOOL("atomic.dn", NvmeCtrl, params.atomic_dn, 0),
+ DEFINE_PROP_UINT16("atomic.awun", NvmeCtrl, params.atomic_awun, 0),
+ DEFINE_PROP_UINT16("atomic.awupf", NvmeCtrl, params.atomic_awupf, 0),
+ DEFINE_PROP_UINT16("atomic.acwu", NvmeCtrl, params.atomic_acwu, 0),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index bed8191bd5fd..66d89bc4bbfb 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -220,6 +220,12 @@ typedef struct NvmeNamespaceParams {
} fdp;
} NvmeNamespaceParams;
+typedef struct NvmeAtomic {
+ uint32_t atomic_max_write_size;
+ QemuMutex atomic_lock;
+ bool atomic_writes;
+} NvmeAtomic;
+
typedef struct NvmeNamespace {
DeviceState parent_obj;
BlockConf blkconf;
@@ -272,6 +278,10 @@ typedef struct NvmeNamespace {
/* reclaim unit handle identifiers indexed by placement handle */
uint16_t *phs;
} fdp;
+ bool atomic_dn;
+ uint16_t atomic_awun;
+ uint16_t atomic_awupf;
+ uint16_t atomic_acwu;
} NvmeNamespace;
static inline uint32_t nvme_nsid(NvmeNamespace *ns)
@@ -421,6 +431,7 @@ typedef struct NvmeRequest {
NvmeCmd cmd;
BlockAcctCookie acct;
NvmeSg sg;
+ bool atomic_write;
QTAILQ_ENTRY(NvmeRequest)entry;
} NvmeRequest;
@@ -537,6 +548,10 @@ typedef struct NvmeParams {
uint8_t sriov_max_vq_per_vf;
uint8_t sriov_max_vi_per_vf;
bool msix_exclusive_bar;
+ uint16_t atomic_awun;
+ uint16_t atomic_awupf;
+ uint16_t atomic_acwu;
+ bool atomic_dn;
} NvmeParams;
typedef struct NvmeCtrl {
@@ -617,6 +632,8 @@ typedef struct NvmeCtrl {
uint16_t vqrfap;
uint16_t virfap;
} next_pri_ctrl_cap; /* These override pri_ctrl_cap after reset */
+ uint32_t dn; /* Disable Normal */
+ NvmeAtomic atomic;
} NvmeCtrl;
typedef enum NvmeResetType {
--
2.39.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC 0/1] hw/nvme: add atomic write support
2024-04-15 23:46 [RFC 0/1] hw/nvme: add atomic write support Alan Adamson
2024-04-15 23:46 ` [RFC 1/1] " Alan Adamson
@ 2024-05-08 10:17 ` Klaus Jensen
1 sibling, 0 replies; 3+ messages in thread
From: Klaus Jensen @ 2024-05-08 10:17 UTC (permalink / raw
To: Alan Adamson; +Cc: qemu-devel, kbusch, qemu-block
[-- Attachment #1: Type: text/plain, Size: 3588 bytes --]
On Apr 15 16:46, Alan Adamson wrote:
> Since there is discussion in the Linux NVMe Driver community to add NVMe Atomic Write
> support, it would be desirable to test it with qemu nvme emulation.
>
> Initially, this RFC will focus on supporting NVMe controller atomic write parameters(AWUN,
> AWUPF, and ACWU) but will be extended to support Namespace parameters (NAWUN, NAWUPF
> and NACWU).
>
> Atomic Write Parameters for NVMe QEMU
> -------------------------------------
> New NVMe QEMU Parameters (See NVMe Specification for details):
> atomic.dn (default off) - Set the value of Disable Normal.
> atomic.awun=UINT16 (default: 0)
> atomic.awupf=UINT16 (default: 0)
> atomic.acwu=UINT16 (default: 0)
>
> qemu command line example:
> qemu-system-x86_64 -cpu host --enable-kvm -smp cpus=4 -no-reboot -m 8192M -drive file=./disk.img,if=ide \
> -boot c -device e1000,netdev=net0,mac=DE:CC:CC:EF:99:88 -netdev tap,id=net0 \
> -device nvme,id=nvme-ctrl-0,serial=nvme-1,atomic.dn=off,atomic.awun=63,atomic.awupf=63,atomic.acwu=0 \
> -drive file=./nvme.img,if=none,id=nvm-1 -device nvme-ns,drive=nvm-1,bus=nvme-ctrl-0 nvme-ns,drive=nvm-1,bus=nvme-ctrl-0
>
> Making Writes Atomic:
> ---------------------
> - Prior to a command being pulled off the SQ and executed, a check is made to see if it
> conflicts "atomically" with a currently executing command.
> - All currently executing commands on the same namespace, across all SQs need to be checked.
> - If an atomic conflict is detected, the command is not started and remains on the queue.
>
> Testing
> -------
> NVMe QEMU Parameters used: atomic.dn=off,atomic.awun=63,atomic.awupf=63,atomic.acwu=0
>
> # nvme id-ctrl /dev/nvme0 | grep awun
> awun : 63
> # nvme id-ctrl /dev/nvme0 | grep awupf
> awupf : 63
> # nvme id-ctrl /dev/nvme0 | grep acwu
> acwu : 0 < Since qemu-nvme doesn't support Compare and Write, this is always zero
> # nvme get-feature /dev/nvme0 -f 0xa
> get-feature:0x0a (Write Atomicity Normal), Current value:00000000
> #
>
> # fio --filename=/dev/nvme0n1 --direct=1 --rw=randwrite --bs=32k --iodepth=256 --name=iops --numjobs=50 --verify=crc64 --verify_fatal=1 --ioengine=libaio
>
> When executed without atomic write support, eventually the following error will be
> observed:
>
> crc64: verify failed at file /dev/nvme0n1 offset 857669632, length 32768
> (requested block: offset=857669632, length=32768, flags=88)
> Expected CRC: 9c87d3539dafdca0
> Received CRC: d521f7ea3b69d2ee
>
> When executed with atomic write support, this error no longer happens.
>
> Questions
> ---------
> AWUN vs AWUPF - Does the nvme emulation need to do treat these differently? Currently the
> larger of the two will be used as the max atomic write size.
>
> Future Work
> -----------
> - Namespace support (NAWUN, NAWUPF and NACWU)
> - Namespace Boundary support (NABSN, NABO, and NABSPF)
> - Atomic Compare and Write Unit (ACWU)
>
> Alan Adamson (1):
> nvme: add atomic write support
>
> hw/nvme/ctrl.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++-
> hw/nvme/nvme.h | 17 ++++++
> 2 files changed, 163 insertions(+), 1 deletion(-)
>
> --
> 2.39.3
>
Hi Alan,
I have no obvious qualms about this. It is clearly useful for driver
testing and verification and does not negatively impact the performance
when this "faked" feature is not enabled.
Acked-by: Klaus Jensen <k.jensen@samsung.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-08 10:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-15 23:46 [RFC 0/1] hw/nvme: add atomic write support Alan Adamson
2024-04-15 23:46 ` [RFC 1/1] " Alan Adamson
2024-05-08 10:17 ` [RFC 0/1] " Klaus 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.