From 4e0ca20a8aa02f26ad3327749de7c10c8ddcb47d Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Tue, 13 Apr 2021 19:30:07 +0800 Subject: [PATCH] idxd: Do not present pci device info in accel_engine_idxd.c module Purpose: We will also support the kernel idxd driver, so we do not need export this feature in the module file. Change-Id: I965e031497920f527962ba187bccd81de6977b8f Signed-off-by: Ziye Yang Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7336 Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot Reviewed-by: Changpeng Liu Reviewed-by: Shuhei Matsumoto --- CHANGELOG.md | 5 ++ include/spdk/idxd.h | 17 +------ lib/idxd/idxd.c | 4 +- lib/idxd/idxd.h | 3 +- lib/idxd/idxd_user.c | 43 ++++++++++++++--- module/accel/idxd/accel_engine_idxd.c | 48 +------------------ test/unit/lib/idxd/idxd_user.c/idxd_user_ut.c | 13 +++++ 7 files changed, 61 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c76c4126..d3cbf94f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## v21.07: (Upcoming Release) +### idxd + +Remove the probe_cb parameter in spdk_idxd_probe function. And remove the definition +of spdk_idxd_probe_cb function pointer. It should be implemented in idxd_user.c. + ## v21.04: ### accel diff --git a/include/spdk/idxd.h b/include/spdk/idxd.h index b7674627e..4ff0e05d8 100644 --- a/include/spdk/idxd.h +++ b/include/spdk/idxd.h @@ -86,26 +86,14 @@ int spdk_idxd_reconfigure_chan(struct spdk_idxd_io_channel *chan); */ typedef void (*spdk_idxd_req_cb)(void *arg, int status); -/** - * Callback for spdk_idxd_probe() enumeration. - * - * \param cb_ctx User-specified opaque value corresponding to cb_ctx from spdk_idxd_probe(). - * \param pci_dev PCI device that is being probed. - * - * \return true to attach to this device. - */ -typedef bool (*spdk_idxd_probe_cb)(void *cb_ctx, struct spdk_pci_device *pci_dev); - /** * Callback for spdk_idxd_probe() to report a device that has been attached to * the userspace IDXD driver. * * \param cb_ctx User-specified opaque value corresponding to cb_ctx from spdk_idxd_probe(). - * \param pci_dev PCI device that was attached to the driver. * \param idxd IDXD device that was attached to the driver. */ -typedef void (*spdk_idxd_attach_cb)(void *cb_ctx, struct spdk_pci_device *pci_dev, - struct spdk_idxd_device *idxd); +typedef void (*spdk_idxd_attach_cb)(void *cb_ctx, struct spdk_idxd_device *idxd); /** * Enumerate the IDXD devices attached to the system and attach the userspace @@ -119,13 +107,12 @@ typedef void (*spdk_idxd_attach_cb)(void *cb_ctx, struct spdk_pci_device *pci_de * * \param cb_ctx Opaque value which will be passed back in cb_ctx parameter of * the callbacks. - * \param probe_cb will be called once per IDXD device found in the system. * \param attach_cb will be called for devices for which probe_cb returned true * once the IDXD controller has been attached to the userspace driver. * * \return 0 on success, -1 on failure. */ -int spdk_idxd_probe(void *cb_ctx, spdk_idxd_probe_cb probe_cb, spdk_idxd_attach_cb attach_cb); +int spdk_idxd_probe(void *cb_ctx, spdk_idxd_attach_cb attach_cb); /** * Detach specified device returned by spdk_idxd_probe() from the IDXD driver. diff --git a/lib/idxd/idxd.c b/lib/idxd/idxd.c index 69bd2a047..5e1fbe1ba 100644 --- a/lib/idxd/idxd.c +++ b/lib/idxd/idxd.c @@ -353,14 +353,14 @@ idxd_device_destruct(struct spdk_idxd_device *idxd) } int -spdk_idxd_probe(void *cb_ctx, spdk_idxd_probe_cb probe_cb, spdk_idxd_attach_cb attach_cb) +spdk_idxd_probe(void *cb_ctx, spdk_idxd_attach_cb attach_cb) { if (g_idxd_impl == NULL) { SPDK_ERRLOG("No idxd impl is selected\n"); return -1; } - return g_idxd_impl->probe(cb_ctx, probe_cb, attach_cb); + return g_idxd_impl->probe(cb_ctx, attach_cb); } void diff --git a/lib/idxd/idxd.h b/lib/idxd/idxd.h index 1fb45a953..9ee8e817c 100644 --- a/lib/idxd/idxd.h +++ b/lib/idxd/idxd.h @@ -181,7 +181,7 @@ struct idxd_wq { struct spdk_idxd_impl { const char *name; void (*set_config)(struct device_config *g_dev_cfg, uint32_t config_num); - int (*probe)(void *cb_ctx, spdk_idxd_probe_cb probe_cb, spdk_idxd_attach_cb attach_cb); + int (*probe)(void *cb_ctx, spdk_idxd_attach_cb attach_cb); void (*destruct)(struct spdk_idxd_device *idxd); uint64_t (*read_8)(struct spdk_idxd_device *idxd, void *portal, uint32_t offset); char *(*portal_get_addr)(struct spdk_idxd_device *idxd); @@ -192,7 +192,6 @@ struct spdk_idxd_impl { }; struct spdk_idxd_device { - struct spdk_pci_device *device; struct spdk_idxd_impl *impl; void *portals; int wq_id; diff --git a/lib/idxd/idxd_user.c b/lib/idxd/idxd_user.c index 16dbb6285..04262f88a 100644 --- a/lib/idxd/idxd_user.c +++ b/lib/idxd/idxd_user.c @@ -46,6 +46,7 @@ struct spdk_user_idxd_device { struct spdk_idxd_device idxd; + struct spdk_pci_device *device; int sock_id; struct idxd_registers registers; void *reg_base; @@ -56,6 +57,8 @@ struct spdk_user_idxd_device { uint32_t perfmon_offset; }; +typedef bool (*spdk_idxd_probe_cb)(void *cb_ctx, struct spdk_pci_device *pci_dev); + #define __user_idxd(idxd) (struct spdk_user_idxd_device *)idxd pthread_mutex_t g_driver_lock = PTHREAD_MUTEX_INITIALIZER; @@ -149,7 +152,7 @@ idxd_unmap_pci_bar(struct spdk_idxd_device *idxd, int bar) } if (addr) { - rc = spdk_pci_device_unmap_bar(idxd->device, 0, addr); + rc = spdk_pci_device_unmap_bar(user_idxd->device, 0, addr); } return rc; } @@ -162,14 +165,14 @@ idxd_map_pci_bars(struct spdk_idxd_device *idxd) uint64_t phys_addr, size; struct spdk_user_idxd_device *user_idxd = __user_idxd(idxd); - rc = spdk_pci_device_map_bar(idxd->device, IDXD_MMIO_BAR, &addr, &phys_addr, &size); + rc = spdk_pci_device_map_bar(user_idxd->device, IDXD_MMIO_BAR, &addr, &phys_addr, &size); if (rc != 0 || addr == NULL) { SPDK_ERRLOG("pci_device_map_range failed with error code %d\n", rc); return -1; } user_idxd->reg_base = addr; - rc = spdk_pci_device_map_bar(idxd->device, IDXD_WQ_BAR, &addr, &phys_addr, &size); + rc = spdk_pci_device_map_bar(user_idxd->device, IDXD_WQ_BAR, &addr, &phys_addr, &size); if (rc != 0 || addr == NULL) { SPDK_ERRLOG("pci_device_map_range failed with error code %d\n", rc); rc = idxd_unmap_pci_bar(idxd, IDXD_MMIO_BAR); @@ -422,12 +425,15 @@ err_reset: static void user_idxd_device_destruct(struct spdk_idxd_device *idxd) { + struct spdk_user_idxd_device *user_idxd = __user_idxd(idxd); + idxd_unmap_pci_bar(idxd, IDXD_MMIO_BAR); idxd_unmap_pci_bar(idxd, IDXD_WQ_BAR); free(idxd->groups); free(idxd->queues); - free(idxd); + spdk_pci_device_detach(user_idxd->device); + free(user_idxd); } struct idxd_enum_ctx { @@ -450,14 +456,37 @@ idxd_enum_cb(void *ctx, struct spdk_pci_device *pci_dev) return -EINVAL; } - enum_ctx->attach_cb(enum_ctx->cb_ctx, pci_dev, idxd); + enum_ctx->attach_cb(enum_ctx->cb_ctx, idxd); } return 0; } + +static bool +probe_cb(void *cb_ctx, struct spdk_pci_device *pci_dev) +{ + struct spdk_pci_addr pci_addr = spdk_pci_device_get_addr(pci_dev); + + SPDK_NOTICELOG( + " Found matching device at %04x:%02x:%02x.%x vendor:0x%04x device:0x%04x\n", + pci_addr.domain, + pci_addr.bus, + pci_addr.dev, + pci_addr.func, + spdk_pci_device_get_vendor_id(pci_dev), + spdk_pci_device_get_device_id(pci_dev)); + + /* Claim the device in case conflict with other process */ + if (spdk_pci_device_claim(pci_dev) < 0) { + return false; + } + + return true; +} + static int -user_idxd_probe(void *cb_ctx, spdk_idxd_probe_cb probe_cb, spdk_idxd_attach_cb attach_cb) +user_idxd_probe(void *cb_ctx, spdk_idxd_attach_cb attach_cb) { int rc; struct idxd_enum_ctx enum_ctx; @@ -518,8 +547,8 @@ idxd_attach(struct spdk_pci_device *device) } idxd = &user_idxd->idxd; + user_idxd->device = device; idxd->impl = &g_user_idxd_impl; - idxd->device = device; pthread_mutex_init(&idxd->num_channels_lock, NULL); /* Enable PCI busmaster. */ diff --git a/module/accel/idxd/accel_engine_idxd.c b/module/accel/idxd/accel_engine_idxd.c index 44a2f742d..64e7133c4 100644 --- a/module/accel/idxd/accel_engine_idxd.c +++ b/module/accel/idxd/accel_engine_idxd.c @@ -58,12 +58,6 @@ enum channel_state { static bool g_idxd_initialized = false; -struct pci_device { - struct spdk_pci_device *pci_dev; - TAILQ_ENTRY(pci_device) tailq; -}; -static TAILQ_HEAD(, pci_device) g_pci_devices = TAILQ_HEAD_INITIALIZER(g_pci_devices); - struct idxd_device { struct spdk_idxd_device *idxd; TAILQ_ENTRY(idxd_device) tailq; @@ -396,38 +390,8 @@ idxd_get_io_channel(void) return spdk_get_io_channel(&idxd_accel_engine); } -static bool -probe_cb(void *cb_ctx, struct spdk_pci_device *pci_dev) -{ - struct spdk_pci_addr pci_addr = spdk_pci_device_get_addr(pci_dev); - struct pci_device *pdev; - - SPDK_NOTICELOG( - " Found matching device at %04x:%02x:%02x.%x vendor:0x%04x device:0x%04x\n", - pci_addr.domain, - pci_addr.bus, - pci_addr.dev, - pci_addr.func, - spdk_pci_device_get_vendor_id(pci_dev), - spdk_pci_device_get_device_id(pci_dev)); - - pdev = calloc(1, sizeof(*pdev)); - if (pdev == NULL) { - return false; - } - pdev->pci_dev = pci_dev; - TAILQ_INSERT_TAIL(&g_pci_devices, pdev, tailq); - - /* Claim the device in case conflict with other process */ - if (spdk_pci_device_claim(pci_dev) < 0) { - return false; - } - - return true; -} - static void -attach_cb(void *cb_ctx, struct spdk_pci_device *pci_dev, struct spdk_idxd_device *idxd) +attach_cb(void *cb_ctx, struct spdk_idxd_device *idxd) { struct idxd_device *dev; @@ -465,7 +429,7 @@ accel_engine_idxd_init(void) return -EINVAL; } - if (spdk_idxd_probe(NULL, probe_cb, attach_cb) != 0) { + if (spdk_idxd_probe(NULL, attach_cb) != 0) { SPDK_ERRLOG("spdk_idxd_probe() failed\n"); return -EINVAL; } @@ -483,7 +447,6 @@ static void accel_engine_idxd_exit(void *ctx) { struct idxd_device *dev; - struct pci_device *pci_dev; if (g_idxd_initialized) { spdk_io_device_unregister(&idxd_accel_engine, NULL); @@ -496,13 +459,6 @@ accel_engine_idxd_exit(void *ctx) free(dev); } - while (!TAILQ_EMPTY(&g_pci_devices)) { - pci_dev = TAILQ_FIRST(&g_pci_devices); - TAILQ_REMOVE(&g_pci_devices, pci_dev, tailq); - spdk_pci_device_detach(pci_dev->pci_dev); - free(pci_dev); - } - spdk_accel_engine_module_finish(); } diff --git a/test/unit/lib/idxd/idxd_user.c/idxd_user_ut.c b/test/unit/lib/idxd/idxd_user.c/idxd_user_ut.c index c7db07d36..c8328c690 100644 --- a/test/unit/lib/idxd/idxd_user.c/idxd_user_ut.c +++ b/test/unit/lib/idxd/idxd_user.c/idxd_user_ut.c @@ -46,6 +46,19 @@ DEFINE_STUB(spdk_pci_idxd_get_driver, struct spdk_pci_driver *, (void), NULL); DEFINE_STUB_V(idxd_impl_register, (struct spdk_idxd_impl *impl)); +DEFINE_STUB_V(spdk_pci_device_detach, (struct spdk_pci_device *device)); +DEFINE_STUB(spdk_pci_device_claim, int, (struct spdk_pci_device *dev), 0); +DEFINE_STUB(spdk_pci_device_get_device_id, uint16_t, (struct spdk_pci_device *dev), 0); +DEFINE_STUB(spdk_pci_device_get_vendor_id, uint16_t, (struct spdk_pci_device *dev), 0); + +struct spdk_pci_addr +spdk_pci_device_get_addr(struct spdk_pci_device *pci_dev) +{ + struct spdk_pci_addr pci_addr; + + memset(&pci_addr, 0, sizeof(pci_addr)); + return pci_addr; +} int spdk_pci_enumerate(struct spdk_pci_driver *driver, spdk_pci_enum_cb enum_cb, void *enum_ctx)