close_peer_eventfds() cleans up three things: ioeventfd triggers if
they exist, eventfds, and the array to store them.
Commit 98609cd (v1.2.0) fixed it not to clean up ioeventfd triggers
when they don't exist (property ioeventfd=off, which is the default).
Unfortunately, the fix also made it skip cleanup of the eventfds and
the array then. This is a memory and file descriptor leak on unplug.
Additionally, the reset of nb_eventfds is skipped. Doesn't matter on
unplug. On peer disconnect, however, this permanently wedges the
interrupt vectors used for that peer's ID. The eventfds stay behind,
but aren't connected to a peer anymore. When the ID gets recycled for
a new peer, the new peer's eventfds get assigned to vectors after the
old ones. Commonly, the device's number of vectors matches the
server's, so the new ones get dropped with a "Too many eventfd
received" message. Interrupts either don't work (common case) or go
to the wrong vector.
Fix by narrowing the conditional to just the ioeventfd trigger
cleanup.
While there, move the "invalid" peer check to the only caller where it
can actually happen, and tighten it to reject own ID.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1458066895-20632-25-git-send-email-armbru@redhat.com>
ivshmem_read() processes server messages. These are 64 bit signed
integers. -1 is shared memory setup, 16 bit unsigned is a peer ID,
anything else is invalid.
ivshmem_read() rejects invalid negative messages right away, silently.
Invalid positive messages get rejected only in resize_peers(), and
ivshmem_read() then prints the rather cryptic message "failed to
resize peers array".
Extend the first check to cover all invalid messages, make it report
"server sent invalid message", and drop the second check.
Now resize_peers() can't fail anymore; simplify.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1458066895-20632-23-git-send-email-armbru@redhat.com>
An interrupt is set up when the interrupt's file descriptor is
received. Each message applies to the next interrupt vector.
Therefore, each vector cannot be set up more than once.
ivshmem_add_kvm_msi_virq() half-heartedly tries not to rely on this by
doing nothing then, but that's not going to recover from this error
should it become possible in the future. watch_vector_notifier()
doesn't even try.
Simply assert what is the case, so we get alerted if we ever screw it
up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1458066895-20632-22-git-send-email-armbru@redhat.com>
The ivshmem device can either use MSI-X or legacy INTx for interrupts.
With MSI-X enabled, peer interrupt events trigger an MSI as they
should. But software can still raise INTx via interrupt status and
mask register in BAR 0. This is explicitly prohibited by PCI Local
Bus Specification Revision 3.0, section 6.8.3.3:
While enabled for MSI or MSI-X operation, a function is prohibited
from using its INTx# pin (if implemented) to request service (MSI,
MSI-X, and INTx# are mutually exclusive).
Fix the device model to leave INTx alone when using MSI-X.
Document that we claim to use INTx in config space even when we don't.
Unlike other devices, ivshmem does *not* use INTx when configured for
MSI-X and MSI-X isn't enabled by software.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1458066895-20632-21-git-send-email-armbru@redhat.com>
There are three predicates related to MSI-X:
* ivshmem_has_feature(s, IVSHMEM_MSI) is true unless the non-MSI-X
variant of the device is selected with msi=off.
* msix_present() is true when the device has the PCI capability MSI-X.
It's initially false, and becomes true during successful realize of
the MSI-X variant of the device. Thus, it's the same as
ivshmem_has_feature(s, IVSHMEM_MSI) for realized devices.
* msix_enabled() is true when msix_present() is true and guest software
has enabled MSI-X.
Code that differs between the non-MSI-X and the MSI-X variant of the
device needs to be guarded by ivshmem_has_feature(s, IVSHMEM_MSI) or
by msix_present(), except the latter works only for realized devices.
Code that depends on whether MSI-X is in use needs to be guarded with
msix_enabled().
Code review led me to two minor messes:
* ivshmem_vector_notify() calls msix_notify() even when
!msix_enabled(), unlike most other MSI-X-capable devices. As far as
I can tell, msix_notify() does nothing when !msix_enabled(). Add
the guard anyway.
* Most callers of ivshmem_use_msix() guard it with
ivshmem_has_feature(s, IVSHMEM_MSI). Not necessary, because
ivshmem_use_msix() does nothing when !msix_present(). That's
ivshmem's only use of msix_present(), though. Guard it
consistently, and drop the now redundant msix_present() check.
While there, rename ivshmem_use_msix() to ivshmem_msix_vector_use().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1458066895-20632-20-git-send-email-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
If pci_ivshmem_realize() fails after it created its migration blocker,
the blocker is left in place. Fix that by creating it last.
Likewise, if it fails after it called fifo8_create(), it leaks fifo
memory. Fix that the same way.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1458066895-20632-18-git-send-email-armbru@redhat.com>
We reuse errp after passing it host_memory_backend_get_memory(). If
both host_memory_backend_get_memory() and the reuse set an error, the
reuse will fail the assertion in error_setv(). Fortunately,
host_memory_backend_get_memory() can't fail.
Pass it &error_abort to make our assumption explicit, and to get the
assertion failure in the right place should it become invalid.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1458066895-20632-17-git-send-email-armbru@redhat.com>
Yes, the chardev is commonly useless after we read a bad version from
it, but destroying it is inappropriate anyway: the user created it, so
the user should be able to hold on to it as long as he likes. We
don't destroy it on other errors. Screwed up in commit 5105b1d.
Stop reading instead.
Also note QEMU's behavior in ivshmem-spec.txt.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1458066895-20632-16-git-send-email-armbru@redhat.com>
IVShmemState member eventfd_chr is useless since commit 9940c32. Drop
it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1458066895-20632-14-git-send-email-armbru@redhat.com>
All references to mr->ram_addr are replaced by
memory_region_get_ram_addr(mr) (except for a few assertions that are
replaced with mr->ram_block).
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1456813104-25902-5-git-send-email-famz@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Simplify the interrupt handling by having a single callback on irq&msi
cases. Remove usage of CharDriver, replace it with
qemu_set_fd_handler(). Use event_notifier_test_and_clear() to read the
eventfd.
Before this patch, ivshmem writes the first byte received to
s->intrstatus. But ivshmem_device_spec.txt says "The status register is
set to 1 when an interrupt occurs." Fortunately, the byte usually comes
from another ivshmem device, and those always write 1.
After this commit, follows the specification, set to 1 when an interrupt
occurs.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Call ivshmem_setup_interrupts() with or without MSI, always allocate
msi_vectors that is going to be used in all case in the following patch.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Fix crash when msi=false introduced in 660c97ee (msi_vectors is NULL in
this case)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1453832250-766-25-git-send-email-peter.maydell@linaro.org
If virtio-net driver allocates memory in ivshmem shared memory,
vhost-net will work correctly, but vhost-user will not work because
a fd of shared memory will not be sent to vhost-user backend.
This patch fixes ivshmem to store file descriptor of shared memory.
It will be used when vhost-user negotiates vhost-user backend.
Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The device's guest interface and its QEMU user interface are
flawed^Whotly debated. We'll resolve that in the next development
cycle, probably by deprecating the device in favour of a cleaned up,
but not quite compatible revision.
To avoid adding more baggage to the soon-to-be-deprecated interface,
mark property "memdev" as experimental, by renaming it to "x-memdev".
It's the only recent user interface change.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1448384789-14830-6-git-send-email-armbru@redhat.com>
[Update of qemu-doc.texi squashed in]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The current ivshmem protocol uses 'long' for integers. But the
sizeof(long) depends on the host and the endianess is not defined, which
may cause portability troubles.
Instead, switch to using little-endian int64_t. This breaks the
protocol, except on x64 little-endian host where this change
should be compatible.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Use irqfd for improving context switch when notifying the guest.
If the host doesn't support kvm irqfd, regular msi notifications are
still supported.
Note: the ivshmem implementation doesn't allow switching between MSI and
IO interrupts, this patch doesn't either.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
The array is used to have vector specific data, so use a more
descriptive name.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
No need to store an extra int for the vector number when it can be
computed easily by looking at the position in the array.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Instead of handling allocation, teach ivshmem to use a memory backend.
This allows to use hugetlbfs backed memory now.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Use the common qemu utility function to parse the memory size.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Remove shm_fd from device state, closing it as early as possible to avoid leaks.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Send a protocol version as the first message from server, clients must
close communication if they don't support this protocol version. Older
QEMUs should be fine with this change in the protocol since they
overrides their own vm_id on reception of an id associated to no
eventfd.
Signed-off-by: David Marchand <david.marchand@6wind.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[use fifo_update_and_get()]
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
The interrupt mask is a state value, it should be reset, like the
interrupt status.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
The number of eventfd that can be handled per peer is limited by the
number of vectors. Return an error when receiving too many of them.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
The terms 'guest' and 'peer' are used sometime interchangeably which may
be confusing. Instead, use 'peer' for the remote instances of ivshmem
clients, and 'guest' for the local VM.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Free all objects owned by the device, making sure the device is free,
fixing hot-unplug.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
The server should not change the shm, and this isn't handled by qemu and
we should should verify this in qemu.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
0 is a valid fd value, so change conditions and set -1 value early
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
load_state_old() is used to keep compatibility with version 0.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
The common version correctly checks for 0 value case.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Both if branches return, move this out to common end.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Use some more explicit variables to simplify the code.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
The server shouldn't send invalid peer id, so print an error if it's the
case.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
The test whether the chardev is an AF_UNIX socket rejects
"-chardev socket,id=chr0,path=/tmp/foo,server,nowait -device
ivshmem,chardev=chr0", but fails to explain why.
Use an explicit error on why a chardev may be rejected.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
max_peer isn't really useful, it tracks the maximum received VM id, but
that quickly matches nb_peers, the size of the peers array. Since VM
come and go, there might be sparse peers so it doesn't help much in
general to have this value around.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
There is no peer when device is initialized, do not let doorbell for
inexisting peer 0.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
val isn't used in ivshmem_update_irq() function.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
It simplifies a bit the code to allocate the array when setting the
number of peers instead of lazily when receiving the first vector.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Set the number of peers and array allocation in a single place. Rename
to better reflect the function content.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Markus Armbruster
Fam Zheng
Marc-André Lureau
Peter Maydell
Tetsuya Mukawa
David Marchand