From 8951c15759049a9898396cf123244412650f5711 Mon Sep 17 00:00:00 2001 From: paul luse Date: Wed, 8 Sep 2021 17:29:35 -0400 Subject: [PATCH] accel/idxd: add and respect flag to support writes to PMEM Plumbing for flags was added in prior pathces. This patch introduces and respects the relevant flags for use with PMEM aka durable memory through the accel_fw, IDXD, IOAT and SW modules. Signed-off-by: paul luse Change-Id: I792f31459e061d220965feced60e0c236d819a68 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9455 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- CHANGELOG.md | 5 + include/spdk/accel_engine.h | 3 + include/spdk/idxd.h | 5 + include/spdk/idxd_spec.h | 4 +- lib/accel/accel_engine.c | 143 ++++++++++++++---- module/accel/idxd/accel_engine_idxd.c | 16 ++ module/accel/ioat/accel_engine_ioat.c | 5 + test/unit/lib/accel/accel.c/accel_engine_ut.c | 7 + 8 files changed, 160 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94b6d1f7b6..ec7581b3e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ Removed deprecated spdk_bdev_module_finish_done(). Use spdk_bdev_module_fini_don A new parameter `flags` was added to all low level submission and preparation APIs to enable the caller to pass operation flags per the DSA specification. +A new flag 'SPDK_IDXD_FLAG_PERSISTENT' was added to let DSA know that +the destination is persistent. + ### accel_fw A new parameter `flags` was added to accel API. @@ -21,6 +24,8 @@ The APIs include: `spdk_accel_submit_copy_crc32c` `spdk_accel_submit_copy_crc32cv` +A new flag `ACCEL_FLAG_PERSISTENT` was added to indicate the target memory is PMEM. + ### bdev_nvme Added `bdev_nvme_add_error_injection` and `bdev_nvme_remove_error_injection` RPCs to add and diff --git a/include/spdk/accel_engine.h b/include/spdk/accel_engine.h index 066923f113..8f1028bcd4 100644 --- a/include/spdk/accel_engine.h +++ b/include/spdk/accel_engine.h @@ -44,6 +44,9 @@ extern "C" { #endif +/* Flags for accel operations */ +#define ACCEL_FLAG_PERSISTENT (1 << 0) + enum accel_capability { ACCEL_COPY = 1 << 0, ACCEL_FILL = 1 << 1, diff --git a/include/spdk/idxd.h b/include/spdk/idxd.h index 9d63311a88..7837bc2de8 100644 --- a/include/spdk/idxd.h +++ b/include/spdk/idxd.h @@ -63,6 +63,11 @@ extern "C" { */ #define SPDK_IDXD_FLAG_NONTEMPORAL IDXD_FLAG_CACHE_CONTROL +/* The following flag is optional and specifies that the destination is persistent memory. The + * low level library will not set this flag. + */ +#define SPDK_IDXD_FLAG_PERSISTENT IDXD_FLAG_DEST_STEERING_TAG + /** * Opaque handle for a single IDXD channel. */ diff --git a/include/spdk/idxd_spec.h b/include/spdk/idxd_spec.h index 9d4a518f2f..d9f5ce60f8 100644 --- a/include/spdk/idxd_spec.h +++ b/include/spdk/idxd_spec.h @@ -84,8 +84,8 @@ extern "C" { #define IDXD_FLAG_COMPLETION_ADDR_VALID (1 << 2) #define IDXD_FLAG_REQUEST_COMPLETION (1 << 3) #define IDXD_FLAG_CACHE_CONTROL (1 << 8) - -#define IDXD_FLAG_CRC_READ_CRC_SEED (1 << 16) +#define IDXD_FLAG_DEST_STEERING_TAG (1 << 15) +#define IDXD_FLAG_CRC_READ_CRC_SEED (1 << 16) /* * IDXD is a family of devices, DSA is the only currently diff --git a/lib/accel/accel_engine.c b/lib/accel/accel_engine.c index a4a6fa7e6a..058e29ce18 100644 --- a/lib/accel/accel_engine.c +++ b/lib/accel/accel_engine.c @@ -43,6 +43,10 @@ #include "spdk/crc32.h" #include "spdk/util.h" +#ifdef SPDK_CONFIG_PMDK +#include "libpmem.h" +#endif + /* Accelerator Engine Framework: The following provides a top level * generic API for the accelerator functions defined here. Modules, * such as the one in /module/accel/ioat, supply the implementation @@ -66,12 +70,12 @@ static void *g_fini_cb_arg = NULL; static TAILQ_HEAD(, spdk_accel_module_if) spdk_accel_module_list = TAILQ_HEAD_INITIALIZER(spdk_accel_module_list); -static void _sw_accel_dualcast(void *dst1, void *dst2, void *src, uint64_t nbytes); -static void _sw_accel_copy(void *dst, void *src, uint64_t nbytes); -static void _sw_accel_copyv(void *dst, struct iovec *iov, uint32_t iovcnt); -static int _sw_accel_compare(void *src1, void *src2, uint64_t nbytes); -static void _sw_accel_fill(void *dst, uint8_t fill, uint64_t nbytes); -static void _sw_accel_crc32c(uint32_t *dst, void *src, uint32_t seed, uint64_t nbytes); +static void _sw_accel_dualcast(void *dst1, void *dst2, void *src, size_t nbytes, int flags); +static void _sw_accel_copy(void *dst, void *src, size_t nbytes, int flags); +static void _sw_accel_copyv(void *dst, struct iovec *iov, uint32_t iovcnt, int flags); +static int _sw_accel_compare(void *src1, void *src2, size_t nbytes); +static void _sw_accel_fill(void *dst, uint8_t fill, size_t nbytes, int flags); +static void _sw_accel_crc32c(uint32_t *dst, void *src, uint32_t seed, size_t nbytes); static void _sw_accel_crc32cv(uint32_t *dst, struct iovec *iov, uint32_t iovcnt, uint32_t seed); /* Registration of hw modules (currently supports only 1 at a time) */ @@ -165,6 +169,20 @@ _add_to_comp_list(struct accel_io_channel *accel_ch, struct spdk_accel_task *acc TAILQ_INSERT_TAIL(&sw_ch->tasks_to_complete, accel_task, link); } +/* Used when the SW engine is selected and the durable flag is set. */ +inline static int +_check_flags(int flags) +{ + if (flags & ACCEL_FLAG_PERSISTENT) { +#ifndef SPDK_CONFIG_PMDK + /* PMDK is required to use this flag. */ + SPDK_ERRLOG("ACCEL_FLAG_PERSISTENT set but PMDK not configured. Configure PMDK or do not use this flag.\n"); + return -EINVAL; +#endif + } + return 0; +} + /* Accel framework public API for copy function */ int spdk_accel_submit_copy(struct spdk_io_channel *ch, void *dst, void *src, uint64_t nbytes, @@ -172,6 +190,7 @@ spdk_accel_submit_copy(struct spdk_io_channel *ch, void *dst, void *src, uint64_ { struct accel_io_channel *accel_ch = spdk_io_channel_get_ctx(ch); struct spdk_accel_task *accel_task; + int rc; accel_task = _get_task(accel_ch, cb_fn, cb_arg); if (accel_task == NULL) { @@ -187,7 +206,11 @@ spdk_accel_submit_copy(struct spdk_io_channel *ch, void *dst, void *src, uint64_ if (_is_supported(accel_ch->engine, ACCEL_COPY)) { return accel_ch->engine->submit_tasks(accel_ch->engine_ch, accel_task); } else { - _sw_accel_copy(dst, src, nbytes); + rc = _check_flags(flags); + if (rc) { + return rc; + } + _sw_accel_copy(dst, src, (size_t)nbytes, flags); _add_to_comp_list(accel_ch, accel_task, 0); return 0; } @@ -200,6 +223,7 @@ spdk_accel_submit_dualcast(struct spdk_io_channel *ch, void *dst1, void *dst2, v { struct accel_io_channel *accel_ch = spdk_io_channel_get_ctx(ch); struct spdk_accel_task *accel_task; + int rc; if ((uintptr_t)dst1 & (ALIGN_4K - 1) || (uintptr_t)dst2 & (ALIGN_4K - 1)) { SPDK_ERRLOG("Dualcast requires 4K alignment on dst addresses\n"); @@ -221,7 +245,11 @@ spdk_accel_submit_dualcast(struct spdk_io_channel *ch, void *dst1, void *dst2, v if (_is_supported(accel_ch->engine, ACCEL_DUALCAST)) { return accel_ch->engine->submit_tasks(accel_ch->engine_ch, accel_task); } else { - _sw_accel_dualcast(dst1, dst2, src, nbytes); + rc = _check_flags(flags); + if (rc) { + return rc; + } + _sw_accel_dualcast(dst1, dst2, src, (size_t)nbytes, flags); _add_to_comp_list(accel_ch, accel_task, 0); return 0; } @@ -249,7 +277,7 @@ spdk_accel_submit_compare(struct spdk_io_channel *ch, void *src1, void *src2, ui if (_is_supported(accel_ch->engine, ACCEL_COMPARE)) { return accel_ch->engine->submit_tasks(accel_ch->engine_ch, accel_task); } else { - rc = _sw_accel_compare(src1, src2, nbytes); + rc = _sw_accel_compare(src1, src2, (size_t)nbytes); _add_to_comp_list(accel_ch, accel_task, rc); return 0; } @@ -262,6 +290,7 @@ spdk_accel_submit_fill(struct spdk_io_channel *ch, void *dst, uint8_t fill, uint { struct accel_io_channel *accel_ch = spdk_io_channel_get_ctx(ch); struct spdk_accel_task *accel_task; + int rc; accel_task = _get_task(accel_ch, cb_fn, cb_arg); if (accel_task == NULL) { @@ -277,7 +306,11 @@ spdk_accel_submit_fill(struct spdk_io_channel *ch, void *dst, uint8_t fill, uint if (_is_supported(accel_ch->engine, ACCEL_FILL)) { return accel_ch->engine->submit_tasks(accel_ch->engine_ch, accel_task); } else { - _sw_accel_fill(dst, fill, nbytes); + rc = _check_flags(flags); + if (rc) { + return rc; + } + _sw_accel_fill(dst, fill, (size_t)nbytes, flags); _add_to_comp_list(accel_ch, accel_task, 0); return 0; } @@ -306,7 +339,7 @@ spdk_accel_submit_crc32c(struct spdk_io_channel *ch, uint32_t *crc_dst, void *sr if (_is_supported(accel_ch->engine, ACCEL_CRC32C)) { return accel_ch->engine->submit_tasks(accel_ch->engine_ch, accel_task); } else { - _sw_accel_crc32c(crc_dst, src, seed, nbytes); + _sw_accel_crc32c(crc_dst, src, seed, (size_t)nbytes); _add_to_comp_list(accel_ch, accel_task, 0); return 0; } @@ -361,6 +394,7 @@ spdk_accel_submit_copy_crc32c(struct spdk_io_channel *ch, void *dst, void *src, { struct accel_io_channel *accel_ch = spdk_io_channel_get_ctx(ch); struct spdk_accel_task *accel_task; + int rc; accel_task = _get_task(accel_ch, cb_fn, cb_arg); if (accel_task == NULL) { @@ -379,8 +413,12 @@ spdk_accel_submit_copy_crc32c(struct spdk_io_channel *ch, void *dst, void *src, if (_is_supported(accel_ch->engine, ACCEL_COPY_CRC32C)) { return accel_ch->engine->submit_tasks(accel_ch->engine_ch, accel_task); } else { - _sw_accel_copy(dst, src, nbytes); - _sw_accel_crc32c(crc_dst, src, seed, nbytes); + rc = _check_flags(flags); + if (rc) { + return rc; + } + _sw_accel_copy(dst, src, (size_t)nbytes, flags); + _sw_accel_crc32c(crc_dst, src, seed, (size_t)nbytes); _add_to_comp_list(accel_ch, accel_task, 0); return 0; } @@ -394,6 +432,7 @@ spdk_accel_submit_copy_crc32cv(struct spdk_io_channel *ch, void *dst, struct iov { struct accel_io_channel *accel_ch; struct spdk_accel_task *accel_task; + int rc; if (src_iovs == NULL) { SPDK_ERRLOG("iov should not be NULL"); @@ -424,7 +463,11 @@ spdk_accel_submit_copy_crc32cv(struct spdk_io_channel *ch, void *dst, struct iov if (_is_supported(accel_ch->engine, ACCEL_COPY_CRC32C)) { return accel_ch->engine->submit_tasks(accel_ch->engine_ch, accel_task); } else { - _sw_accel_copyv(dst, src_iovs, iov_cnt); + rc = _check_flags(flags); + if (rc) { + return rc; + } + _sw_accel_copyv(dst, src_iovs, iov_cnt, flags); _sw_accel_crc32cv(crc_dst, src_iovs, iov_cnt, seed); _add_to_comp_list(accel_ch, accel_task, 0); return 0; @@ -596,41 +639,89 @@ sw_accel_get_capabilities(void) return 0; } -static void -_sw_accel_dualcast(void *dst1, void *dst2, void *src, uint64_t nbytes) +static inline void +_pmem_memcpy(void *dst, const void *src, size_t len) { - memcpy(dst1, src, (size_t)nbytes); - memcpy(dst2, src, (size_t)nbytes); +#ifdef SPDK_CONFIG_PMDK + int is_pmem = pmem_is_pmem(dst, len); + + if (is_pmem) { + pmem_memcpy_persist(dst, src, len); + } else { + memcpy(dst, src, len); + pmem_msync(dst, len); + } +#else + SPDK_ERRLOG("Function not defined without SPDK_CONFIG_PMDK enabled.\n"); + assert(0); +#endif } static void -_sw_accel_copy(void *dst, void *src, uint64_t nbytes) +_sw_accel_dualcast(void *dst1, void *dst2, void *src, size_t nbytes, int flags) { - memcpy(dst, src, (size_t)nbytes); + if (flags & ACCEL_FLAG_PERSISTENT) { + _pmem_memcpy(dst1, src, nbytes); + _pmem_memcpy(dst2, src, nbytes); + } else { + memcpy(dst1, src, nbytes); + memcpy(dst2, src, nbytes); + } } static void -_sw_accel_copyv(void *dst, struct iovec *iov, uint32_t iovcnt) +_sw_accel_copy(void *dst, void *src, size_t nbytes, int flags) +{ + + if (flags & ACCEL_FLAG_PERSISTENT) { + _pmem_memcpy(dst, src, nbytes); + } else { + memcpy(dst, src, nbytes); + } +} + +static void +_sw_accel_copyv(void *dst, struct iovec *iov, uint32_t iovcnt, int flags) { uint32_t i; for (i = 0; i < iovcnt; i++) { assert(iov[i].iov_base != NULL); - memcpy(dst, iov[i].iov_base, iov[i].iov_len); + if (flags & ACCEL_FLAG_PERSISTENT) { + _pmem_memcpy(dst, iov[i].iov_base, (size_t)iov[i].iov_len); + } else { + memcpy(dst, iov[i].iov_base, (size_t)iov[i].iov_len); + } dst += iov[i].iov_len; } } static int -_sw_accel_compare(void *src1, void *src2, uint64_t nbytes) +_sw_accel_compare(void *src1, void *src2, size_t nbytes) { - return memcmp(src1, src2, (size_t)nbytes); + return memcmp(src1, src2, nbytes); } static void -_sw_accel_fill(void *dst, uint8_t fill, uint64_t nbytes) +_sw_accel_fill(void *dst, uint8_t fill, size_t nbytes, int flags) { - memset(dst, fill, nbytes); + if (flags & ACCEL_FLAG_PERSISTENT) { +#ifdef SPDK_CONFIG_PMDK + int is_pmem = pmem_is_pmem(dst, nbytes); + + if (is_pmem) { + pmem_memset_persist(dst, fill, nbytes); + } else { + memset(dst, fill, nbytes); + pmem_msync(dst, nbytes); + } +#else + SPDK_ERRLOG("Function not defined without SPDK_CONFIG_PMDK enabled.\n"); + assert(0); +#endif + } else { + memset(dst, fill, nbytes); + } } static void diff --git a/module/accel/idxd/accel_engine_idxd.c b/module/accel/idxd/accel_engine_idxd.c index 66939498de..fe4bd420c1 100644 --- a/module/accel/idxd/accel_engine_idxd.c +++ b/module/accel/idxd/accel_engine_idxd.c @@ -158,9 +158,17 @@ _process_single_task(struct spdk_io_channel *ch, struct spdk_accel_task *task) siov.iov_len = task->nbytes; diov.iov_base = task->dst; diov.iov_len = task->nbytes; + if (task->flags & ACCEL_FLAG_PERSISTENT) { + flags |= SPDK_IDXD_FLAG_PERSISTENT; + flags |= SPDK_IDXD_FLAG_NONTEMPORAL; + } rc = spdk_idxd_submit_copy(chan->chan, &diov, 1, &siov, 1, flags, idxd_done, task); break; case ACCEL_OPCODE_DUALCAST: + if (task->flags & ACCEL_FLAG_PERSISTENT) { + flags |= SPDK_IDXD_FLAG_PERSISTENT; + flags |= SPDK_IDXD_FLAG_NONTEMPORAL; + } rc = spdk_idxd_submit_dualcast(chan->chan, task->dst, task->dst2, task->src, task->nbytes, flags, idxd_done, task); break; @@ -175,6 +183,10 @@ _process_single_task(struct spdk_io_channel *ch, struct spdk_accel_task *task) memset(&task->fill_pattern, fill_pattern, sizeof(uint64_t)); diov.iov_base = task->dst; diov.iov_len = task->nbytes; + if (task->flags & ACCEL_FLAG_PERSISTENT) { + flags |= SPDK_IDXD_FLAG_PERSISTENT; + flags |= SPDK_IDXD_FLAG_NONTEMPORAL; + } rc = spdk_idxd_submit_fill(chan->chan, &diov, 1, task->fill_pattern, flags, idxd_done, task); break; @@ -203,6 +215,10 @@ _process_single_task(struct spdk_io_channel *ch, struct spdk_accel_task *task) } diov.iov_base = task->dst; diov.iov_len = task->nbytes; + if (task->flags & ACCEL_FLAG_PERSISTENT) { + flags |= SPDK_IDXD_FLAG_PERSISTENT; + flags |= SPDK_IDXD_FLAG_NONTEMPORAL; + } rc = spdk_idxd_submit_copy_crc32c(chan->chan, &diov, 1, iov, iovcnt, task->seed, task->crc_dst, flags, idxd_done, task); diff --git a/module/accel/ioat/accel_engine_ioat.c b/module/accel/ioat/accel_engine_ioat.c index 9acfb3c92e..9508bde575 100644 --- a/module/accel/ioat/accel_engine_ioat.c +++ b/module/accel/ioat/accel_engine_ioat.c @@ -139,6 +139,11 @@ ioat_submit_tasks(struct spdk_io_channel *ch, struct spdk_accel_task *accel_task struct spdk_accel_task *tmp; int rc = 0; + if (accel_task->flags == ACCEL_FLAG_PERSISTENT) { + SPDK_ERRLOG("IOAT does not support durable destinations.\n"); + return -EINVAL; + } + do { switch (accel_task->op_code) { case ACCEL_OPCODE_MEMFILL: diff --git a/test/unit/lib/accel/accel.c/accel_engine_ut.c b/test/unit/lib/accel/accel.c/accel_engine_ut.c index 1d01b4bbbd..06eef7ccde 100644 --- a/test/unit/lib/accel/accel.c/accel_engine_ut.c +++ b/test/unit/lib/accel/accel.c/accel_engine_ut.c @@ -39,6 +39,13 @@ #include "accel/accel_engine.c" #include "unit/lib/json_mock.c" +#ifdef SPDK_CONFIG_PMDK +DEFINE_STUB(pmem_msync, int, (const void *addr, size_t len), 0); +DEFINE_STUB(pmem_memcpy_persist, void *, (void *pmemdest, const void *src, size_t len), NULL); +DEFINE_STUB(pmem_is_pmem, int, (const void *addr, size_t len), 0); +DEFINE_STUB(pmem_memset_persist, void *, (void *pmemdest, int c, size_t len), NULL); +#endif + /* global vars and setup/cleanup functions used for all test functions */ struct spdk_accel_engine g_accel_engine = {}; struct spdk_io_channel *g_ch = NULL;