From 32bff28a2527963b889e9751ba8f7612e38b37b8 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 2 Nov 2017 15:00:20 -0700 Subject: [PATCH] nvme: use spdk_pci_device_claim() in nvme_pcie_ctrlr_construct spdk_pci_device_claim() can be used to ensure only one process at a time uses any given PCI device. Previously this was only used in the bdev_nvme driver - other apps like nvme/perf do not use spdk_pci_device_claim() and could effectively rip out the device from a running bdev-based app like the NVMe-oF target. So instead of modifying all of the nvme apps, put this logic into the core nvme driver instead so that all applications get the benefit transparently. Save the fd when the controller is constructed and then close it when the controller is destructed to handle the detach (including hotplug) cases. Signed-off-by: Jim Harris Change-Id: I5dc48a2e41dc06707800f15a9e1f9141477628c6 Reviewed-on: https://review.gerrithub.io/385524 Reviewed-by: Dariusz Stojaczyk Reviewed-by: Ben Walker Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp --- lib/bdev/nvme/bdev_nvme.c | 9 -------- lib/nvme/nvme_pcie.c | 22 +++++++++++++++++-- test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c | 6 +++++ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/lib/bdev/nvme/bdev_nvme.c b/lib/bdev/nvme/bdev_nvme.c index d4fa82a54..cff765272 100644 --- a/lib/bdev/nvme/bdev_nvme.c +++ b/lib/bdev/nvme/bdev_nvme.c @@ -721,7 +721,6 @@ probe_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, } if (trid->trtype == SPDK_NVME_TRANSPORT_PCIE) { - struct spdk_pci_addr pci_addr; bool claim_device = false; struct nvme_probe_ctx *ctx = cb_ctx; size_t i; @@ -737,14 +736,6 @@ probe_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, SPDK_DEBUGLOG(SPDK_TRACE_BDEV_NVME, "Not claiming device at %s\n", trid->traddr); return false; } - - if (spdk_pci_addr_parse(&pci_addr, trid->traddr)) { - return false; - } - - if (spdk_pci_device_claim(&pci_addr) < 0) { - return false; - } } return true; diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index a0b053872..f99779ac1 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -36,7 +36,7 @@ */ #include "spdk/stdinc.h" - +#include "spdk/env.h" #include "spdk/likely.h" #include "nvme_internal.h" #include "nvme_uevent.h" @@ -87,6 +87,9 @@ struct nvme_pcie_ctrlr { /* Opaque handle to associated PCI device. */ struct spdk_pci_device *devhandle; + /* File descriptor returned from spdk_pci_device_claim(). Closed when ctrlr is detached. */ + int claim_fd; + /* Flag to indicate the MMIO register has been remapped */ bool is_remapped; }; @@ -660,8 +663,20 @@ struct spdk_nvme_ctrlr *nvme_pcie_ctrlr_construct(const struct spdk_nvme_transpo struct nvme_pcie_ctrlr *pctrlr; union spdk_nvme_cap_register cap; uint32_t cmd_reg; - int rc; + int rc, claim_fd; struct spdk_pci_id pci_id; + struct spdk_pci_addr pci_addr; + + if (spdk_pci_addr_parse(&pci_addr, trid->traddr)) { + SPDK_ERRLOG("could not parse pci address\n"); + return NULL; + } + + claim_fd = spdk_pci_device_claim(&pci_addr); + if (claim_fd < 0) { + SPDK_ERRLOG("could not claim device %s\n", trid->traddr); + return NULL; + } pctrlr = spdk_dma_zmalloc(sizeof(struct nvme_pcie_ctrlr), 64, NULL); if (pctrlr == NULL) { @@ -674,6 +689,7 @@ struct spdk_nvme_ctrlr *nvme_pcie_ctrlr_construct(const struct spdk_nvme_transpo pctrlr->ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_PCIE; pctrlr->devhandle = devhandle; pctrlr->ctrlr.opts = *opts; + pctrlr->claim_fd = claim_fd; memcpy(&pctrlr->ctrlr.trid, trid, sizeof(pctrlr->ctrlr.trid)); rc = nvme_pcie_ctrlr_allocate_bars(pctrlr); @@ -765,6 +781,8 @@ nvme_pcie_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr) struct nvme_pcie_ctrlr *pctrlr = nvme_pcie_ctrlr(ctrlr); struct spdk_pci_device *devhandle = nvme_ctrlr_proc_get_devhandle(ctrlr); + close(pctrlr->claim_fd); + if (ctrlr->adminq) { nvme_pcie_qpair_destroy(ctrlr->adminq); } diff --git a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c index dfdf5ad0c..49305a4a5 100644 --- a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c +++ b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c @@ -152,6 +152,12 @@ spdk_pci_device_cfg_write32(struct spdk_pci_device *dev, uint32_t value, uint32_ abort(); } +int +spdk_pci_device_claim(const struct spdk_pci_addr *pci_addr) +{ + abort(); +} + int nvme_ctrlr_construct(struct spdk_nvme_ctrlr *ctrlr) {