This change introduces initial experimental wrappers for enabling/
disabling rte_pci_device interrupts and for getting event file
descriptor assosiated with an interrupt.
Signed-off-by: Maciej Szulik <maciej.szulik@intel.com>
Change-Id: Iba1ba1e57a3555001502859d0bb2c655c07bf956
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10502
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
These APIs are not safe, since they do not hold the
pci device lock across calls, which can cause problems
if a device is inserted or removed while handles
returned by these APIs are being used.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I01a80f26d0a0ca4cdfc7181359932b38da8dd43a
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10659
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
This is a safer alternative to spdk_pci_get_first/next_device,
since those APIs do not hold the lock between calls.
Future patches will remove those APIs, and change callers to
use this new API instead.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I71c7e8c1feb9112da8be32a8056b30e105e30463
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10655
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
DPDK will report error when detaching the PCI device in secondary
process, because SPDK will return -1 in `pci_device_fini`, so
here we will reset the `attached` flag before that.
Also return the errno instead of -1.
Change-Id: I3efa4d97ceab504215faeb9d3d80a694bdd6014c
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7944
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Dong Yi <dongx.yi@intel.com>
DPDK recently clarified some semantics on the rte_devargs 'data'
and 'args' fields. This actually breaks our use of the 'data'
field to store the 2 second timeout timestamp for delaying
attach to newly inserted devices. Investigating this further,
it does not seem our use of the 'data' field was valid - it just
happened to work until now.
We could use the 'args' field now. But knowing whether to use
'args' or 'data' would then be dependent on the DPDK version.
We cannot use RTE_VERSION_NUM to decide, because this is a
compile time decision, and it is possible in shared library
use cases that we could actually link and execute against a
different version of DPDK than we built against.
So instead we will create our own env_devargs structure that
will store these allowed_at timestamps. Currently it's just
a linked list (which is exactly how DPDK does it) - we could
make it more optimal with a hash table down the road, but this
code only executes when we are doing PCI enumeration so it is
not performance critical.
Fixes#1904.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I3ee5d65ba90635b5a96b97dd0f4ab72a093fe8f7
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7506
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: <dongx.yi@intel.com>
The dbdf format is xxxx:xx:xx.x and with the wrong
format the rte_devargs_parse always fails.
Change-Id: Ia34bc5e68f6401bb25907d5d07c65636b4f491b5
Signed-off-by: Jin Yu <jin.yu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7140
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Community-CI: Mellanox Build Bot
Can not remove device in the remove event
callback as we can not unregister the remove
callback. So use the alarm_set to fix this issue.
Fixes#1809
Change-Id: Ib86bc4eeecc0fe2bc51538e28684d015405e8835
Signed-off-by: Jin Yu <jin.yu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6553
Reviewed-by: Vasuki Manikarnike <vasuki.manikarnike@hpe.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: <dongx.yi@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Provide a default stub definition for spdk_pci_device_claim/unclaim
for non-linux platforms, rather than just for FreeBSD.
Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
Change-Id: Ica45d967878582d9a58e37b088eba4bf0d94104e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6464
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Community-CI: Mellanox Build Bot
The env layer has a pci_allowed list, which specifies
that only a subset of PCI devices may be attached
by the associated process.
But that doesn't cover PCI devices that are hot-inserted
after the application starts, which is common for
storage/NVMe.
So add a new spdk_pci_device_allow() API which allows
an application to add new devices to the allowed list.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I7bd5ff428d84480d46bc236698daadd019b20b8e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6183
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: <dongx.yi@intel.com>
RTE_DEV_ALLOWED is an enum and has no associated define, hence checking
for its presence will always be false.
We could test for RTE_DEV_WHITELISTED define, but this macro added for
deprecation warning will be dropped in the future.
Switch to a check on DPDK version.
Fixes: 10ed0eb755 ("env_dpdk/pci: adapt to 20.11 EAL changes")
Signed-off-by: David Marchand <david.marchand@redhat.com>
Change-Id: I75270977b580065b36c753266cbaa5fb73f99eb1
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5165
Community-CI: Mellanox Build Bot
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
DPDK 20.11 renamed device and bus control enums [1].
This is a simple renaming, no change in semantics.
1: https://git.dpdk.org/dpdk/commit/?id=a65a34a85ebf
Signed-off-by: David Marchand <david.marchand@redhat.com>
Change-Id: Ia40bae750ad74f405eb700b47514fca021ffd052
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5116
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Move the lock files from '/tmp' to '/var/tmp' cause user maybe delete files in /tmp
or remount /tmp by mistake, And the JSON-RPC domain socket located in '/var/tmp' also.
Signed-off-by: Weifeng Su <suweifeng1@huawei.com>
Signed-off-by: Shihao Sun <sunshihao@huawei.com>
Change-Id: I18d52f42462e8477fb35aeea9e38efc51610d17c
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5096
Community-CI: Broadcom CI
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
And modify test/env/vtophys to resolve linking errors.
SPDK_PRINTF() and SPDK_ERRLOG() use spdk_log() procedure which is
customizable and redirectable, so it is preffered over printf()
In case of test/env/vtophys/ program,
we have to make it an app first to avoid linking errors.
Change-Id: Id806ec3bb235745316063bbdf6b5a15a9d5dc2d9
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1944
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Give this a more generic name. We're going to be using these
events for more than just hotremove coming up.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Change-Id: Ia5356e9ab809807ba4d85ecc212a496e96012bce
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3559
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Now that drivers can be registered from upper layers there's
no need to keep them centralized inside env.
(check_format.sh complains that spdk_pci_nvme_get_driver() shouldn't
start with the spdk_ prefix - to workaround that we move the function
declaration from one place in env.h to another - that's enough to
convince check_format it really is a public function)
Change-Id: If86aebd6c997349569c71430ec815b413eb44ef8
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3187
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This allows SPDK apps to register new PCI drivers outside of
the env layer, enabling SPDK as a whole with new use cases.
Change-Id: I0c998a9ec249c3ca610b7b3b8b6caf616b16f64c
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3185
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
An alternative to spdk_pci_*_get_driver();
Change-Id: I20a80b3c655a37fb1c76da21c2b70d5678041fab
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3186
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
We want to make struct spdk_pci_driver public, so add env-agnostic
fields that define a driver directly to that struct.
PCI driver registration (potentially in upper layers) will only use
spdk_pci_id-s and spdk pci drv_flags, then those will be translated
to DPDK equivalents inside env_dpdk.
Change-Id: Ia24ecfc99ebf0f54f096eaf27bca5ed9c0dfe01d
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3183
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
With this patch, spdk_app_start/stop can be repeatedly
called by users based on their upper level application's
requirement.
Changes are:
* Add reinit ability inside spdk_env_init and related functions
* Clear g_shutdown_sig_received in spdk_app_setup_signal_handlers
* Clear malloc_disk_count in bdev_malloc_initialize
Change-Id: I2d7be52b0e4aac2cb6734cc1237ce72d33b6de0c
Signed-off-by: Xiaodong Liu <xiaodong.liu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2260
Community-CI: Mellanox Build Bot
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Now that we support only DPDK 18.11+ and always have
to register pci drivers to DPDK on initialization we
don't need that flag - it's always true.
Change-Id: Ibf1d79155595609fe9093f58e056bea25db6fdb2
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3446
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This follows struct rte_pci_id which had class_id as well.
We'll need it to make some additional DPDK APIs public through
the env abstraction.
Change-Id: I794a6cd6b17e48daf53b48fa5abe3d3dcfeaa403
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3182
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
You don't get notified when someone starts using your hooked
device, so there's not much gain from knowing when someone
stops.
Remove that callback and also move DPDK device detach under
the same lock which sets the pending_removal flag. This eliminates
a data race window when hotremove notification could arrive
after device was detached, but before it was scheduled to be
removed.
vmd and ioat nest the spdk_pci_device struct and abigail complains
even though the parent structs only have forward declarations in
public headers. Adding those two structs to the suppression list
doesn't help though. Abidiff still complains about the pci device
struct being changed, probably because ioat.h and vmd.h both include
env.h. Abidiff suppresion list should eventually be split per-lib,
but for now ignore struct spdk_pci_device changes globally.
$ abidiff [...]/libspdk_ioat.so [...]
'struct spdk_pci_device at env.h:652:1' changed:
type size changed from 1024 to 960 (in bits)
1 data member deletion:
<SNIP>
Change-Id: I9b113572c661f0e0786b6d625e16dc07fe77e778
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2939
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
A workaround for kernel deadlocks surfaced in #1275.
DPDK basically offers two APIs for hotplugging all PCI devices:
rte_bus_scan() and rte_bus_probe(). Scan iterates through
/sys/bus/pci/devices/* and creates corresponding rte_pci_device-s,
then rte_bus_probe() tries to initialize each device with the
supporting driver.
Previously we did scan and probe together, one after another, now
we'll have an intermediate step. After scanning the bus, we'll
iterate through all rte_pci_device-s and temporarily blacklist any
newly detected devices. We'll use devargs->data field to a store
a timeout value (integer) after which the device can be un-blacklisted
and initialized. devargs->data is documented in DPDK as "Device
string storage" and it's a char*, but it's not referenced anywhere
in DPDK. rte_bus_probe() respects the blacklist and doesn't do
absolutely anything with blacklisted ones.
The timeout value is 2 seconds, which should be plenty enough
for an NVMe device to reset, leave the critical lock sections in
kernel, and let us initialize it safely.
Note that direct attach by BDF doesn't respect the blacklist,
so an NVMe attach RPC won't be delayed in any way, it will continue
to work as it always did. Only the automatic discovery & enumeration
is deferred.
Change-Id: I62b719271bd0755bc2882331ea33f69897b1e5e5
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1733
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Extensive testing showed it can fail:
> EAL: eal_parse_sysfs_value(): cannot open sysfs value
> /sys/bus/pci/devices/0000:02:00.0/vendor
> EAL: Scan for (pci) bus failed.
spdk_pci_enumerate() would previously return with error because
of this and e.g. the test nvme hotplug app could immediately exit
with failure. A mis-timed scan shouldn't cause this kind of failure,
so ignore it's return code. This shouldn't cause any issues.
Change-Id: I9253219c218981a747774a8632335963cfb0db53
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2941
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: <dongx.yi@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
There was a chance we scheduled a device removal to the DPDK thread
while that thread was already removing the device from a VFIO hotremove
notification (on the DPDK interrupt thread). The second hotremove
attempt touches some freed memory and segfaults.
The VFIO hotremove notification already checks pending_removal flag
under a mutex and sets it to true, so do the same in spdk_detach_rte()
(called from the SPDK init thread).
Change-Id: Ib3f0eb7c0c5c6e1ab8cf253b7711fd149925a143
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1730
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Simplify the code path a bit. VFIO notification is the only
place where detach callback is called from the dpdk intr thread.
Detach checks the current thread and behaves differently in this
case, but it could be the VFIO notification that simply calls
a different function.
So instead of carrying the VFIO notification through the generic
detach routine, carry it just through the DPDK-thread specific
subset. This lets us remove some ifs in the generic routine.
Change-Id: I5e8866e4643ef08fb3cd12621e2d262b5e827c74
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1731
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This reverts commit 301c5aeec9.
The patch doesn't fix anything as the hotremoval could be still
called twice and the second call would do use-after-free.
Change-Id: I78a1120707dbdf36c871ec378a312c4a058fc76b
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1729
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Hitting only the static functions from the above libraries
with the spdk_ prefix.
Signed-off-by: Seth Howell <seth.howell@intel.com>
Change-Id: Ic6df38dfbeb53f0b1c30d350921f7216acba3170
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2362
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
When removing large number of devices (>8) in parallel,
the 20ms timeout is not long enough.
As part of spdk_detach_cb, DPDK calls into the VFIO driver
which may get delayed due to multiple hot removes being
processed by pciehp driver (pciehp IRQ thread function
is handling the actual removal of a device in paralle but
all of the IRQ thread function compete for a global mutex
increasing processing time and race conditions).
Signed-off-by: Michael Haeuptle <michael.haeuptle@hpe.com>
Change-Id: I470fbbee92dac9677082c873781efe41e2941cd5
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1588
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
We use `spdk_map_bar_rte()` to read mapped addresses
from PCI BARs.
This function is currently checking for NULL in each pair.
But in PCI memory, some registers can be left unused,
in which case they are set to 0.
As a result, we may read some NULL pointers from BARs,
which is OK.
To check if given address is indeed invalid, we should first
check if it is used.
So it is best to delegate such checks to the
user of this function.
In fact, users already do the NULL check where it is needed
(ex: virtio_pci.c:390, nvme_pcie.c:589)
so this patch just removes them from `spdk_map_bar_rte()`.
This solves github issue #1206
Change-Id: I88021ceca1b9e9d503b224f790819999cd16da01
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1129
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
spdk_map_bar_rte did not return error in case bar was not mapped successfully
Signed-off-by: Lukasz Radomski <lukasz.radomski@intel.com>
Change-Id: I662cc189d47c65af8f135a3ab4b27ff1785233d0
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/477812
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
DPDK intr thread is designed that it can't unregister the src
callback in this callback handler. So I think we can't detach
the PCI device in the hotremove callback as it needs to unregister
the VFIO notification callback which will be not successful
but it still can free the device. So at the next req notification
in the handler function, we meet the freed device.
Fix#994
Change-Id: Id4b45a2d0fe6b45b132355d59471bc80240fad70
Signed-off-by: Jin Yu <jin.yu@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473176
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
The function allows the user to get string representation of the type of
a PCI device.
Change-Id: I02abcd9fc98ba912ca4d7936be22e9d5b4950ea2
Signed-off-by: Konrad Sztyber <konrad.sztyber@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470648
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Wojciech Malikowski <wojciech.malikowski@intel.com>
spdk_pci_device_claim() could create a file on the
filesystem that couldn't be deleted programatically.
It could only be overwritten - e.g. by another spdk
instance - but this didn't really work if that
another instance had less privileges and hence no
access to the previous file.
This is exactly the case we're seeing on our CI when
running SPDK as non-root. In general it's a good idea
not to leave any leftover files, so now we'll delete
the pci claim file when the spdk process exits.
spdk_pci_device_claim() used to return a file descriptor
that could be simply closed to "un-claim" the device.
It'll now return only a return code. The fd will be
stored inside spdk_pci_device and will be closed either
when user calls the newly introduced spdk_pci_device_unclaim(),
or when the device is detached.
We'll still need to clean up those files somewhere in
our test scripts (probably ./setup.sh cleanup) to
clean up after crashed processes or so - but we don't
necessarily want to run such scripts inside the autotest
whenever a non-root spdk is about to be started.
Change-Id: I797e079417bb56491013cc5b92f0f0d14f451d18
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/467107
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
By making dpdk device detach asynchronous we have
actually broken some cases where devices are re-attached
immediately after and fail since they were not detached
yet, so now we're making device detach synchronous again.
For that we'll simply wait inside spdk_pci_device_detach()
for the background dpdk thread to perform all necessary
actions before we return. We'll also print an error msg
if DPDK failed the detach (probably because of some
internal error).
Change-Id: I7657ac1b169169eae3325de2d28c2cc311e7d901
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/460286
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: <jacek.kalwas@intel.com>
By making dpdk device detach asynchronous we have
actually broken some cases where devices are re-attached
immediately after and fail since they were not detached
yet.
We'll need to make detach synchronous again, and for that
we'll wait for the background dpdk thread to perform all
necessary actions before we return from spdk_pci_device_detach().
However, device detach could be triggered from the very
same dpdk background thread as well. Waiting there would
cause a deadlock, so now we'll schedule asynchronous
device detach to the dpdk thread only if we're not on
that thread already.
This patch itself serves also as an optimization.
Change-Id: I86b7ac1b669169eee3325de2d28c2cc313e7d901
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/460285
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Added spdk_pci_get_first_device() and
spdk_pci_get_next_device() to iterate
over all devices on g_pci_devices list.
Change-Id: I65079fb3e274195707dee64bc1fb8b4b72d07352
Signed-off-by: Wojciech Malikowski <wojciech.malikowski@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450924
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Put the locks inside cleanup_pci_devices().
This serves as cleanup.
Change-Id: I040b28006e5584d1f33af26b63cafedbafe04fdb
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458934
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: John Kariuki <John.K.Kariuki@intel.com>
The global pci tailq is no longer modified on the dpdk
thread, so on the spdk thread we can access it safely
without any lock. The code is slightly more readable
then.
This shows that cleanup_pci_devices() is always wrapped
with lock/unlock. We'll put the locks inside this
function in the next patch.
Change-Id: Ia4d386b78a87078761df0a3b953bfc4ff44102f8
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458933
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
To safely access the global pci device list on an spdk
thread, we'll need not to modify this list on any other
thread. When device gets hotplugged on a dpdk thread,
it will be now inserted into a new global tailq that
can be accessed only under g_pci_mutex. Then any
subsequently called public pci function will add it to
the regular device tailq.
Change-Id: I9cb9d6b24fd731641fd764d0da71bedab38824c9
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458932
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
To safely access the global pci device list on an spdk
thread, we'll need not to modify this list on any other
thread. When device gets hotremoved on a dpdk thread,
it will now set a new per-device `removed` flag. Then
any subsequently called public pci function will remove
it from the list.
Change-Id: I0f16237617e0bea75b322ab402407780616424c3
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458931
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
For VMD driver we'll need to introduce some way of
iterating over all spdk pci device objects and we would
like to achieve that with simple spdk_pci_get_first_dev()/get_next_dev()
APIs. To make it thread safe though, we would have to
expose some public pci mutex to be locked around the
iteration and we don't want to do that, so we'll make
PCI APIs usable from only a single thread - this will
prevent any pci devices from being removed inbetween
subsequent get_first/get_next calls.
We currently have the following players accessing pci
device state:
1) public APIs, obviously (on any thread right now)
2) VFIO hotremove callback (dpdk interrupt thread)
3) rte_eal_alarm for detaching rte_pci_devices (dpdk
interrupt thread)
4) DPDK hotplug IPC (dpdk interrupt thread)
There is g_pci_mutex providing the thread safety, but
even today it doesn't protect #3 and #4, making the
entire pci layer prone to data corruption.
To make #3 and #4 safe, we would have to lock inside
device init/fini callbacks (spdk_pci_device_init/fini),
but those are called directly inside the public device
attach/detach functions which already lock.
So now, with the decision to drop thread safety from
public pci APIs, we narrow down the locks inside public
functions and introduce locks inside those lower-level
init/fini callbacks.
Change-Id: I5dcbc9cdcbab65ee76cd3c42890f596069ec9a8a
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458930
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
While detaching the device, DPDK may try to unregister
a VFIO interrupt callback which is currently "in use".
The unregister call may fail, but the error doesn't get
propagated to upper DPDK layers. Practically, detaching
the device may stop in the middle but still return 0 to
SPDK.
This effectively breaks hotremove as the device would
be neither usable or removable.
We work around it in SPDK by internally scheduling the
DPDK device detach on the DPDK interrupt thread. This
prevents any other interrupt callback to be "in use"
while the device is detached.
Since device detach in SPDK can be asynchronous now,
we add a few checks to prevent re-attaching devices
that are still being detached.
Change-Id: Ibb56a8017e34418db0304fe32774811427b056aa
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448928
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This is an attempt to fix device hotremove with VFIO.
A soft device hotremove request through sysfs [1] would
currently just block until the SPDK process manually
releases that device - e.g. upon an RPC request.
VFIO won't get unbound from the device untill userspace
releases all its resources. VFIO can signal a pending
hotremove request by kicking any file descriptor provided
by the userspace - and DPDK does provide such descriptor -
but SPDK does not listen on it.
DPDK does offer handy API to listen and in this patch
we make use of it inside our env/pci layer. Within
a DPDK callback we set an internal per-device hotremove
flag, which upper-layer SPDK drivers can poll with a new
env API - spdk_pci_device_is_removed().
The VFIO hotremove event will be sent to primary
processes only, so that's where we listen.
We make use of this new API in the NVMe hotplug poller,
which will process it just like any other supported
hotremove event.
Fixes#595Fixes#690
[1] # echo 1 > /sys/bus/pci/devices/<bdf>/remove
Change-Id: I03d88271c2089c740e232056d9340e5a640d442c
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448927
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
It's mostly needed for the next patch, but even
now it provides some value by printing errors if
there any leaked (still attached) PCI devices
at shutdown.
Change-Id: I8459a6049b3c6612d9f1d99444bf3acfd474a839
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/449082
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
