Commit Graph

55 Commits

Author SHA1 Message Date
Darek Stojaczyk
12f622b769 virtio: zero-out virtqueue structures at init
They used to be allocated with spdk_dma_zmalloc() which did
provide zeroed memory, but we unintentionally changed that
when switching to posix_memalign.

The structure might have some unitialized memory, so with
this patch we just memset it right after allocating it.

Change-Id: Id5a5685e09419901513925abaeed605c36f5199a
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/451546
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2019-04-22 16:49:39 +00:00
Darek Stojaczyk
01103b2e4d virtio: allocate virtqueue structs using regular calloc
The DMA-able vrings are allocated separately, so
the general virtqueue object can be allocated with
regular malloc - it only contains some local PMD
context.

While here, also allocate those DMA-able vrings using
spdk_zmalloc() instead of spdk_dma_zmalloc(), as
spdk_dma_*malloc() is about to be deprecated.

Change-Id: I06b9e0256c14c21747c253f05b63ef2361f465c7
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450550
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-04-17 22:53:05 +00:00
Darek Stojaczyk
794b2533de virtio: shrink the process-local virtqueue structure
We used to ceil the size of the vring structure to the
nearest cache line boundary. That's how the original
DPDK implementation behaved, but I can't find a reason
for this. This patch gets rid of the ceiling.

Change-Id: Iaa40fdb79c60252237901f77023ff2f9e580eece
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450549
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
2019-04-09 06:30:42 +00:00
Darek Stojaczyk
a86029b967 virtio/pci: don't rely on phys_addr retrieved from spdk_malloc()
The phys_addr param in spdk_*malloc() is about to be
deprecated, so use a separate spdk_vtophys() call to
retrieve physical addresses.

This patch also adds error checks against SPDK_VTOPHYS_ERROR.
The error handling paths are already there to account for
spdk_*malloc() failures themselves, so reuse them in case
of vtophys failures.

Change-Id: I6d71a4c361f5ec1770d96676f3bb8f57ecd62716
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448167
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-03-20 01:06:09 +00:00
Darek Stojaczyk
e6c2ff37e5 virtio/user: fix memory hotplug with the internal rte_vhost lib
This was broken in commit 2725b5191e [1], which added
an extra #ifdef SPDK_CONFIG_VHOST_INTERNAL_LIB without
including config.h anywhere within the same file.

Currently the virtio library always behaves as if it was
compiled with upstream rte_vhost. Fix this by including
spdk/config.h where necessary.

[1] virtio/user: don't send SET_VRING_ADDR after updating
    the memory table

Change-Id: Idadc53e20d615ca1dcef5b58a854f6b39d14511b
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448222
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>
2019-03-19 04:02:37 +00:00
Darek Stojaczyk
2725b5191e virtio/user: don't send SET_VRING_ADDR after updating the memory table
Our internal rte_vhost lib requires SET_VRING_ADDR to
flush a pending SET_MEM_TABLE. On the other hand, the
upstream rte_vhost will invalidate the entire queue
upon receiving SET_VRING_ADDR message, so we mustn't
send it at runtime. Both behaviors are strictly
implementation specific, but this message isn't needed
by the spec, so send it only if vhost was compiled
with our internal lib.

Change-Id: I200b99a1dd5124bdebd104e69ae1b012a974ab6f
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446087
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2019-03-13 14:26:20 +00:00
Darek Stojaczyk
89c1dc774e virtio: export spec-defined structures into a separate header
Those will be reused by the vhost target and
its message handling hooks.

Change-Id: If41118e3700f3b445312bb691c493bd13134546b
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446084
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2019-03-13 14:26:20 +00:00
Darek Stojaczyk
e67743162f virtio/user: minor cleanup
Make struct vhost_user_msg match the format defined in
the vhost-user spec. The custom additions were inhereted
from DPDK, but SPDK never made use of them. Also replace
direct sizeof(*msg) usages with a handy macro that was
already there. Finally add a VHOST_USER prefix to the
VHOST_MEMORY_MAX_NREGIONS #define.

Change-Id: Idef60fdc62a9e96f076059d60aad796fcc9d3e1c
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446083
Tested-by: SPDK CI Jenkins <sys_sgci@intel.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: Ben Walker <benjamin.walker@intel.com>
2019-03-13 14:26:20 +00:00
Shuhei Matsumoto
ab0f787e0b virtio: Use spdk_json_write_named_* APIs throughout
Change-Id: I0e7f432e5ce8477e34907ba249f2a4e0d85e875e
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/442942
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
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>
2019-02-04 07:08:04 +00:00
Darek Stojaczyk
148eda65c1 virtio: don't reinitialize entire virtqueues on mem hotplug
One of the messages we send on memory hotplug event is
SET_VRING_BASE, which tells vhost e.g. the position in
a vring it should start processing requests from. Sending
this message with any outstanding I/O could cause that
I/O to be never processed as it could be at a vring
position that won't be practically polled.

To fix the above, we don't send SET_VRING_BASE message
on memory hotplug event anymore since it's completely
unnecessary. It was sent together with a couple other
messages that would reinitialize the vring, but we know
vrings occupy a memory buffer that won't be hotremoved
during vring lifetime. We also know that vring GPAs will
never change. Hence we can initialize the vrings just
once on device start now.

We still need to send SET_VRING_ADDR after updating the
memory table, as rte_vhost depends on it to apply that
new memory table. Luckily, this single message doesn't
cause us any trouble.

Change-Id: I2125099f1cf3f8c76e8160ec819bd1a9a3e7823c
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/439436
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2019-01-28 18:56:52 +00:00
Darek Stojaczyk
eecc6dc8e6 memory: replace all hardcoded 0x200000 with a define
Makes the code slightly more readable.

Change-Id: Iebf8fb07bceacf433d4bdad0a30419a3faab7eee
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/439370
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2019-01-13 00:47:26 +00:00
Darek Stojaczyk
8c4ed83b49 vtophys: add length parameter to the vtophys function
This follows the same trend as the mem_map APIs.

Currently, most of the spdk_vtophys() callers manually
detect physically noncontiguous buffers to split them
into multiple physically contiguous chunks. This patch
is a first step towards encapsulating most of that logic
in a single place - in spdk_vtophys() itself.

This patch doesn't change any functionality on its own,
it only extends the API.

Change-Id: I16faa9dea270c370f2a814cd399f59055b5ccc3d
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/438449
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: wuzhouhui <wuzhouhui@kingsoft.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-01-09 08:24:59 +00:00
Darek Stojaczyk
76a7517caa pci: remove virtio-specific attach functions
As a part of cleanup they're replaced by a device-agnostic
attach API, which is easier for us to manage.

Change-Id: I2ec68f20ba209f02ee5c2de4b6fe5330a4bc0853
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/436480
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2018-12-07 17:24:19 +00:00
Dariusz Stojaczyk
23670d8db7 virtio: support dynamic memory registrations
We need to resend SET_MEM_TABLE whenever memory
is hotplugged or hotremoved.

Change-Id: I9bdc975f11b07866122bc49b9cf372071c5370f2
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/422582
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
2018-10-23 18:02:12 +00:00
Dariusz Stojaczyk
c9e5777ede virtio: clarify feature negotiation failure
Change-Id: Ifb1566033569af0b3cf4a9dfbc8094a9883c7622
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/420980
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-08-01 19:16:54 +00:00
Dariusz Stojaczyk
eeb1650c06 virtio: propagate error codes
Many functions were inconsistent in regard of error codes
they returned. In a couple of places we could print "Operation
not permitted" via strerror() from functions that were
supposed to return negative return codes, but returned
`-1` instead. (-EPERM == -1)

Change-Id: I7c36e54d449352dbd8036746e4f44a65c9b1d0b3
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/419007
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-08-01 19:16:54 +00:00
Dariusz Stojaczyk
117fdae94b virtio: fix false-positive assertion failure on virtqueue_req_add_iovs
We need to decrement the free_cnt before checking
if it reached 0.

Note: the vq_free_cnt overflow is asserted at the
beginning of the function.

Change-Id: I655217785e4425d1cd3c704d24321b643be55dcf
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/420584
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
2018-08-01 17:25:24 +00:00
Dariusz Stojaczyk
65165d795d virtio: fix vq init error handling
We didn't handle vq alloc or init failure, because
queues are initialized all at once on device init and
if one vq fails, all of them are to be destroyed.

This behavior is really unintuitive, and with the
latest changes we have a possible segfault scenario.
(We could spdk_dma_free() a buffer that failed to
allocate).

It is now required that the queue allocation function
cleans up after itself.

Change-Id: I6cd1d30c710eb9266288905ab982db363f972a1d
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/419001
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2018-07-11 21:02:06 +00:00
Dariusz Stojaczyk
e8762a41db virtio/pci: make sure each queue is physically contiguous
The spdk_dma_zmalloc guarantee about physical memory contiguity
is about to be removed soon. For hardware rings that require
physical memory or IOVA contiguity we will now enforce hugepage
alignment and size restrictions to make sure they occupy only
a single hugepage.

Change-Id: I8f44ad6f33d60f01403cc3db693497e4f722e528
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/418612
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2018-07-11 21:02:06 +00:00
Dariusz Stojaczyk
095f355bf3 virtio/user: do not set vring physical address
It's not required. We only use it in virtio-pci.

Change-Id: I61e95d680d00fa3d56ebccbc9a372db7e1db296d
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417002
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-07-06 21:57:47 +00:00
Dariusz Stojaczyk
3a15b49b05 virtio: move queue allocation to backend code
Right now both virtio-pci and virtio-user allocate
the queue via spdk_dma_malloc, but that's about
to change soon.

Change-Id: I3acdad45cd9a0639f9070bc448fdf8f9d2c706c0
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417000
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
2018-07-06 19:16:16 +00:00
Dariusz Stojaczyk
e724452ec0 virtio: call backend-specific queue cleanup on vdev stop
This was omitted in the original implementation and the
`del_queue` callback wasn't called anywhere. For virtio-user
we open some eventfds on queue creation and until now they
were never being closed.

Change-Id: Iee1ced1e17a59d5cb13449538c115678a1c1a328
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/416999
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2018-07-06 19:16:16 +00:00
Dariusz Stojaczyk
436c0c189b virtio: check F_CONFIG feature before sending GET/SET_CONFIG
The device might not have this message implemented.
Sending it could result in a connection being terminated.

Change-Id: I53c08f1108ebc7de630569f3983c317cc6510fa4
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417636
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
2018-07-05 20:47:38 +00:00
Dariusz Stojaczyk
62844ae3ca virtio: allow config read/write to fail
For vhost-user it's a protocol feature that can simply
be not supported. The subsequent patch introduces an extra
check that may cause config read/write to fail.

Change-Id: I5b0e11845fb6021472c608477f1797dada8ab961
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417458
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
2018-07-05 20:47:38 +00:00
Dariusz Stojaczyk
48322f0c37 virtio/user: negotiate VHOST_USER_PROTOCOL_F_CONFIG
It was introduced a while after the original GET/SET_CONFIG
implementation in QEMU, but within the same QEMU release (2.12?).
It is required by the vhost-user spec. Rte_vhost doesn't check
it, so everything worked so far, but other implementations
might (and should) reject our GET_CONFIG requests right now.

As a part of this feature, we should also check the same
flag before sending GET/SET_CONFIG messages to respect those
devices that really don't implement F_CONFIG. This is done
in a separate patch.

Change-Id: Ib7e9b11a0074f4aee70609af0cad2ef59a8bf427
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417459
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2018-07-05 20:47:38 +00:00
Dariusz Stojaczyk
0b9e4c8ff7 virtio/user: negotiate VHOST_USER_PROTOCOL_F_MQ
Required by the vhost-user spec.

Change-Id: Id7143a0f6cc34463ad5f22d8db96ac5c51e04081
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417457
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
2018-07-05 20:47:38 +00:00
Dariusz Stojaczyk
5910855c68 virtio/user: support protocol features
rte_vhost doesn't respect those, but any other
implementation should.

Change-Id: Id0a0fa031b7c6e9d572cdffeeb3a1e40d824826d
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417456
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
2018-07-05 20:47:38 +00:00
Dariusz Stojaczyk
f0501959a0 virtio/user: remove leftover kernel vhost references
We don't support the kernel vhost. Vhost-SCSI is not even
fully implemented in Linux, so there's no point trying.

Change-Id: Ie564d46c497718081dd2a5c28829fdcf88e1c0a0
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417455
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-07-02 19:20:08 +00:00
Dariusz Stojaczyk
03b12a98b2 bdev/virtio: support event index
This fixes intermittent failures with QEMU's virtio-scsi-pci
device.

Apparently, from time to time QEMU enters a broken state in
which it doesn't turn off the NO_NOTIFY flag. This flag should
be set only for the period of time virtqueues are being processed,
but QEMU makes it set all the time. This makes us not signal any
I/O submissions - SPDK thinks the device is currently processing
notifications so it will pick up our I/O automatically, but in
realitly it won't and we end up with a deadlock.

I believe kernel virtio driver doesn't hit this issue because of
event index feature. If negotiated, the NO_NOTIFY flag won't be
used at all. Initial tests show that the issue is indeed gone
with this patch.

Event index for SPDK virtio is not particularly useful when using
a poll-mode device, but it doesn't do any harm. It can be further
optimized in the future, but macro-benchmarks don't show any
performance difference compared with the legacy notification
handling so there's no hurry.

Change-Id: I46e8ab0da170780fcdc0dd239208670ee5ed6357
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/415591
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
2018-07-02 19:17:13 +00:00
Dariusz Stojaczyk
587002f511 virtio: expose virtio descriptors as soon as possible
With upcoming event index patch we will need
to keep track of how many descriptor chains we
have really put into the avail vring. This patch
is a step towards that.

Our virtio layer abstracts away descriptor chains
as "requests". We can start requests, add descriptors
to them, and finally flush them. So far we used to put
any descriptors directly into the virtqueue desc ring,
and made them visible to the device only through
virtqueue_req_flush().

All of our virtio bdev modules currently flush the
virtqueue after adding each single request, but the
docs for the virtio API say it's possible to start
multiple subsequent requests and flush them all at
once. This was conceptually broken so far and only
the last request would be exposed to the device.

It's now fixed and subsequent requests are put
into the avail vring as soon as they're complete
(either the next request is started, or the
virtqueue is flushed).

Change-Id: I76b7db88ab094c38430edd8ff0e65681775dcb31
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/415590
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
2018-07-02 19:17:13 +00:00
Dariusz Stojaczyk
8c6bd59704 virtio: remove unnecessary if
This `if` came from the original DPDK virtio PMD.
Generally, we do not want to trigger cache coherency
updates if we don't have to, but in this particular
case we know we always do.

If there were no changes to the avail index, there
would be no requests started and the function would
return much earlier.

Change-Id: Ic1231cf82288c1cb95dc89346f54d51849b8bae9
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/415589
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
2018-06-18 22:31:32 +00:00
Dariusz Stojaczyk
ba43426c3d virtio: remove DEBUGLOG about received descriptors
Once enabled, this DEBUGLOG would be printed
every poller tick. It's not practical, so it's
being removed now.

Change-Id: I8627dedcc2c0df8065ffe575059938d620491dd5
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/415588
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
2018-06-18 22:31:32 +00:00
Dariusz Stojaczyk
b5ad869cc1 virtio: merge contiguous memory regions
Although contiguous, memory regions may appear as separate
/proc/self/maps entries.

This patch brings support for DPDK 18.05.

Change-Id: I91eb8adc2c073a103f687320ec7b9dabe1e066b3
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/413167
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2018-06-01 20:14:32 +00:00
Ben Walker
a00b6819a1 bdev/virtio: Abort on bad index in virtio_dev_queue_get_thread
There is no way to recover from this.

Change-Id: I1667b032bab867d58ad23fa8b1bd59f81620b442
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/408246
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
2018-04-19 15:48:36 -04:00
Dariusz Stojaczyk
0a1a4ce116 virtio: rename ops->get_queue_num callback
It actually returns the size of given queue,
so renamed it to get_queue_size to clean up
the API

Change-Id: I88551116b3dc19644764bba78b58444802a1d443
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/408174
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
2018-04-18 12:32:00 -04:00
Pawel Wodkowski
1a6dac405b bdev/virito: add JSON config dump and generic create RPC
The new construct_virtio_dev allow creating virtio SCSI and blk for both
PCI and user transports.

Change-Id: Ibd79c4fb75e3cbd993b46227d86e915c1b740a18
Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-on: https://review.gerrithub.io/405419
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2018-04-17 17:16:06 -04:00
Dariusz Stojaczyk
3cb775b9ae virtio: don't silence vhost-user errors
All errors were being silenced in an upper virtio layer.
This patch makes them properly propagate further.

Change-Id: I289c810ff0f670c4ba1fbcf0804089639fa561a8
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/405918
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-04-03 15:22:49 -04:00
Dariusz Stojaczyk
91bdec96c7 virtio/vhost_user: set NEEDS_RESET status bit if necessary
This protects against brute-force initialization that virtio
is not really prepared for. Most importantly, this behavior
makes us Virtio spec compliant (3.1 Device Init.).

Change-Id: I6d26f3d8b5cce488a71068777531b78538489662
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/405917
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-04-03 15:22:49 -04:00
Dariusz Stojaczyk
9728d6b054 virtio: check if setting status bits succeeded
This is required by the Virtio spec. (3.1 Device Init.)
It will also help us with vhost-user error handling.
set_status() doesn't have a way to return an error, so
its internal failures were silenced until now. When we
exceeded max memory region count (8) we only printed
an error message without failing any SPDK initialization.
This patch fixes that. It also removes not-particularly-
useful comments in virtio_dev_reset().

Change-Id: Ibe0010c493ef4e12e1fdd0a1679bf706f298d97e
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/405915
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
2018-03-30 19:34:22 -04:00
Dariusz Stojaczyk
6ccc262022 virtio: remove g_virtio_driver
g_virtio_driver contained just a list of all
Virtio-SCSI devices. It's now being removed for
two reasons:
 * it's backend-specific, doesn't fit a generic virtio lib
 * it's difficult to ensure thread-safety for it

Virtio bdev modules will now manage their Virtio
devices by themselves. Virtio-SCSI has now an internal
device list. (And VirtioBlk module maps devices to
bdevs 1:1, so doesn't need any additional work here.)

Change-Id: I3bc68d76d6904df5c56f00fca7ab387871ecf88f
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/405179
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
2018-03-27 00:58:14 -04:00
Dariusz Stojaczyk
59d901ad72 virtio/user: implement get/set config messages
This will let us e.g. read device blocksize in the
upcoming vhost-blk initiator. All the public API
was already there - we've been using it for
virtio-pci. Now we're making use of it for vhost-user
as well.

Change-Id: I39eab820bb9bbff59c8b8efa79cc97d2ec7806fd
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/398828
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-02-23 15:37:11 -05:00
Dariusz Stojaczyk
face9eb258 virtio: add mb() before checking notify flag
The kernel vhost target enables notifying after processing
each interrupt in a following manner.

 * unset NO_NOTIFY flag
 * mb()
 * check avail ring for slipped requests

And we do the following after issuing each I/O.

 * update avail ring
 * check NO_NOTIFY

If NO_NOTIFY check is reordered and read first, we might read
old `true` value, and avail ring might be updated after the
kernel has already done its check. This easily leads to deadlock.

Change-Id: I6bb4490775dfed6fb2987e97c39b713054ae26ad
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/396499
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
2018-02-06 19:27:46 -05:00
Dariusz Stojaczyk
5926647156 virtio: inline internal functions
These didn't make much sense anyway.
Removed them so there's less code

Change-Id: I3247c640b13e5847b5f6d822072b63cad269ac8c
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/398467
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
2018-02-06 19:27:46 -05:00
Dariusz Stojaczyk
7f0c2bf4b4 virtio: remove virtio_dev_get_status asserts
virtio_dev_get_status may result in an MMIO read,
which is quite expensive. Removed it to improve
performance on debug builds.

Change-Id: Ibe2d13ad381f02f8d258d630283c14ce20b7a340
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/398468
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-02-05 19:06:11 -05:00
Dariusz Stojaczyk
fb12bbecd2 virtio: move vdev->name allocation to generic virtio
Previously vdev->name was being allocated/freed
separately in virtio_pci and virtio_user backends.
Now it's all done in generic virtio library and
cleans up some code.

Change-Id: I810e976d09781c0c9b25c6f7fd957a83aad6c7b8
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/394704
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-01-23 22:49:21 -05:00
Dariusz Stojaczyk
e6da08c28a virtio/pci: detach pci device on virtio-pci destroy
Fix memory leak on virtio-pci device destruct.

Change-Id: I7d33802bc15c15d6a2dbac3e83456c9cf23b6004
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/394703
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-01-23 22:49:21 -05:00
Dariusz Stojaczyk
37fb62224a bdev/virtio: add API to attach particular PCI devices
This is just the API introduction. See subsequent
patches for its RPC usage.

Change-Id: Iadb7c9bf6a56ab4330c9f2215c6006a2935d208d
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/394445
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-01-22 15:57:11 -05:00
Dariusz Stojaczyk
5c0c104b81 virtio: add opaque ctx param to PCI enumerate callback
In current bdev_virtio_scsi PCI enumerate callback
implementation we rely on a global variable - a global
list of virtio devices. We do not need any opaque
context data inside this callback just yet. It will
be required to add virtio devices in runtime. See the
next patch for details.

Change-Id: I116cbd3bd633f56922eedcc7c07b8c0310e51d49
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/394444
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2018-01-16 19:13:32 -05:00
Pawel Wodkowski
891c12a63c util: add spdk_strerror() wrapper with TLS support
This patch remove need for additional buffer when translating error code
to string.

Change-Id: Iaa60088b5c450581d3cdddbb425119b17d55a44b
Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/386114
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-01-04 15:00:09 -05:00
Dariusz Stojaczyk
8b94174a4e virtio: fix full-virtqueue handling
Distinguish between "not enough descriptors available"
and "iovcnt > queue depth".

This patch brings back I/O re-queueing for virtio bdev
module. It was temporarily disabled by 451de7e1 [1].

[1] 451de7e1: "virtio: switch to the new virtqueue API"

Change-Id: I4c4f6a6d9d91373ee647ea7cafd53ad999aa6aa2
Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/393447
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2018-01-03 14:35:36 -05:00