On 02.10.2015 21:09, marcandre.lur...@redhat.com wrote:
> From: David Marchand <david.march...@6wind.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.march...@6wind.com>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> [use fifo_update_and_get()]
> ---
> contrib/ivshmem-client/ivshmem-client.c | 13 ++++++++++---
> contrib/ivshmem-client/ivshmem-client.h | 1 +
> contrib/ivshmem-server/ivshmem-server.c | 9 +++++++++
> contrib/ivshmem-server/ivshmem-server.h | 1 +
> docs/specs/ivshmem_device_spec.txt | 9 ++++++---
> hw/misc/ivshmem.c | 31 +++++++++++++++++++++++++++++--
> include/hw/misc/ivshmem.h | 25 +++++++++++++++++++++++++
> 7 files changed, 81 insertions(+), 8 deletions(-)
> create mode 100644 include/hw/misc/ivshmem.h
>
> diff --git a/contrib/ivshmem-client/ivshmem-client.c
> b/contrib/ivshmem-client/ivshmem-client.c
> index 34a65b1..8a7cbfd 100644
> --- a/contrib/ivshmem-client/ivshmem-client.c
> +++ b/contrib/ivshmem-client/ivshmem-client.c
> @@ -206,10 +206,17 @@ ivshmem_client_connect(IvshmemClient *client)
> goto err_close;
> }
>
> - /* first, we expect our index + a fd == -1 */
> + /* first, we expect a protocol version */
> + if (ivshmem_client_read_one_msg(client, &tmp, &fd) < 0 ||
> + (tmp != IVSHMEM_PROTOCOL_VERSION) || fd != -1) {
> + IVSHMEM_CLIENT_DEBUG(client, "cannot read from server\n");
> + goto err_close;
> + }
> +
> + /* then, we expect our index + a fd == -1 */
> if (ivshmem_client_read_one_msg(client, &client->local.id, &fd) < 0 ||
> client->local.id < 0 || fd != -1) {
> - IVSHMEM_CLIENT_DEBUG(client, "cannot read from server\n");
> + IVSHMEM_CLIENT_DEBUG(client, "cannot read from server (2)\n");
> goto err_close;
> }
> IVSHMEM_CLIENT_DEBUG(client, "our_id=%ld\n", client->local.id);
> @@ -221,7 +228,7 @@ ivshmem_client_connect(IvshmemClient *client)
> if (fd >= 0) {
> close(fd);
> }
> - IVSHMEM_CLIENT_DEBUG(client, "cannot read from server (2)\n");
> + IVSHMEM_CLIENT_DEBUG(client, "cannot read from server (3)\n");
> goto err_close;
> }
> client->shm_fd = fd;
> diff --git a/contrib/ivshmem-client/ivshmem-client.h
> b/contrib/ivshmem-client/ivshmem-client.h
> index 284c4a3..9215f34 100644
> --- a/contrib/ivshmem-client/ivshmem-client.h
> +++ b/contrib/ivshmem-client/ivshmem-client.h
> @@ -23,6 +23,7 @@
> #include <sys/select.h>
>
> #include "qemu/queue.h"
> +#include "hw/misc/ivshmem.h"
>
> /**
> * Maximum number of notification vectors supported by the client
> diff --git a/contrib/ivshmem-server/ivshmem-server.c
> b/contrib/ivshmem-server/ivshmem-server.c
> index 4a25d28..060f414 100644
> --- a/contrib/ivshmem-server/ivshmem-server.c
> +++ b/contrib/ivshmem-server/ivshmem-server.c
> @@ -101,6 +101,15 @@ ivshmem_server_send_initial_info(IvshmemServer *server,
> IvshmemServerPeer *peer)
> {
> int ret;
>
> + /* send our protocol version first */
> + ret = ivshmem_server_send_one_msg(peer->sock_fd,
> IVSHMEM_PROTOCOL_VERSION,
> + -1);
> + if (ret < 0) {
> + IVSHMEM_SERVER_DEBUG(server, "cannot send version: %s\n",
> + strerror(errno));
> + return -1;
> + }
> +
> /* send the peer id to the client */
> ret = ivshmem_server_send_one_msg(peer->sock_fd, peer->id, -1);
> if (ret < 0) {
> diff --git a/contrib/ivshmem-server/ivshmem-server.h
> b/contrib/ivshmem-server/ivshmem-server.h
> index e9b0e7a..65b3c2d 100644
> --- a/contrib/ivshmem-server/ivshmem-server.h
> +++ b/contrib/ivshmem-server/ivshmem-server.h
> @@ -32,6 +32,7 @@
> #include <stdbool.h>
>
> #include "qemu/queue.h"
> +#include "hw/misc/ivshmem.h"
>
> /**
> * Maximum number of notification vectors supported by the server
> diff --git a/docs/specs/ivshmem_device_spec.txt
> b/docs/specs/ivshmem_device_spec.txt
> index 12f338e..3435116 100644
> --- a/docs/specs/ivshmem_device_spec.txt
> +++ b/docs/specs/ivshmem_device_spec.txt
> @@ -64,6 +64,8 @@ It creates a shared memory object then waits for clients to
> connect on a unix
> socket.
>
> For each client (QEMU process) that connects to the server:
> +- the server sends a protocol version, if client does not support it, the
> client
> + closes the communication,
> - the server assigns an ID for this client and sends this ID to him as the
> first
> message,
> - the server sends a fd to the shared memory object to this client,
> @@ -86,9 +88,10 @@ been provided in qemu.git/contrib/ivshmem-client for debug.
>
> *QEMU as an ivshmem client*
>
> -At initialisation, when creating the ivshmem device, QEMU gets its ID from
> the
> -server then makes it available through BAR0 IVPosition register for the VM to
> -use (see 'PCI device registers' subsection).
> +At initialisation, when creating the ivshmem device, QEMU first receives a
> +protocol version and closes communication with server if it does not match.
> +Then, QEMU gets its ID from the server then makes it available through BAR0
> +IVPosition register for the VM to use (see 'PCI device registers'
> subsection).
> QEMU then uses the fd to the shared memory to map it to BAR2.
> eventfds for all other clients received from the server are stored to
> implement
> BAR0 Doorbell register (see 'PCI device registers' subsection).
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 1b58010..3b6acd6 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -27,6 +27,8 @@
> #include "qemu/fifo8.h"
> #include "sysemu/char.h"
>
> +#include "hw/misc/ivshmem.h"
> +
> #include <sys/mman.h>
> #include <sys/types.h>
> #include <limits.h>
> @@ -597,6 +599,31 @@ static void ivshmem_read(void *opaque, const uint8_t
> *buf, int size)
> }
> }
>
> +static void ivshmem_check_version(void *opaque, const uint8_t * buf, int
> size)
> +{
> + IVShmemState *s = opaque;
> + int tmp;
> + long version;
> +
> + if (!fifo_update_and_get(s, buf, size,
> + &version, sizeof(version))) {
> + return;
> + }
> +
> + tmp = qemu_chr_fe_get_msgfd(s->server_chr);
> + if (tmp != -1 || version != IVSHMEM_PROTOCOL_VERSION) {
> + fprintf(stderr, "incompatible version, you are connecting to a
> ivshmem-"
> + "server using a different protocol please check your
> setup\n");
> + qemu_chr_delete(s->server_chr);
> + s->server_chr = NULL;
> + return;
> + }
> +
> + IVSHMEM_DPRINTF("version check ok, switch to real chardev handler\n");
> + qemu_chr_add_handlers(s->server_chr, ivshmem_can_receive, ivshmem_read,
> + ivshmem_event, s);
> +}
> +
> /* Select the MSI-X vectors used by device.
> * ivshmem maps events to vectors statically, so
> * we just enable all vectors on init and after reset. */
> @@ -770,8 +797,8 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error
> **errp)
>
> s->eventfd_chr = g_malloc0(s->vectors * sizeof(CharDriverState *));
>
> - qemu_chr_add_handlers(s->server_chr, ivshmem_can_receive,
> ivshmem_read,
> - ivshmem_event, s);
> + qemu_chr_add_handlers(s->server_chr, ivshmem_can_receive,
> + ivshmem_check_version, ivshmem_event, s);
> } else {
> /* just map the file immediately, we're not using a server */
> int fd;
> diff --git a/include/hw/misc/ivshmem.h b/include/hw/misc/ivshmem.h
> new file mode 100644
> index 0000000..433ef53
> --- /dev/null
> +++ b/include/hw/misc/ivshmem.h
> @@ -0,0 +1,25 @@
> +
> +/*
> + * Inter-VM Shared Memory PCI device.
> + *
> + * Author:
> + * Cam Macdonell <c...@cs.ualberta.ca>
> + *
> + * Based On: cirrus_vga.c
> + * Copyright (c) 2004 Fabrice Bellard
> + * Copyright (c) 2004 Makoto Suzuki (suzu)
> + *
> + * and rtl8139.c
> + * Copyright (c) 2006 Igor Kovalenko
> + *
> + * This code is licensed under the GNU GPL v2.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + */
> +#ifndef IVSHMEM_H
> +#define IVSHMEM_H
> +
> +#define IVSHMEM_PROTOCOL_VERSION 0
> +
> +#endif /* IVSHMEM_H */
>
Reviewed-by: Claudio Fontana <claudio.font...@huawei.com>
