Re: [PATCH v6 3/3] ivshmem: add check on protocol version in QEMU

2014-09-25 Thread David Marchand

On 09/23/2014 05:58 PM, Stefan Hajnoczi wrote:

I'm not sure a full-fledged feature negotiation system is needed.  The
ivshmem protocol is local to the host and all participants are under
control of the administrator.

I suggested a protocol version to protect against misconfiguration.  For
example, building QEMU from source but talking to an outdated ivhsmem
server that is still running from before.

Remember that ivshmem-server and QEMU are shipped together by the
distro.  So in 99% of the cases they will have the same version anyway.
But we want to protect against rare misconfiguration that break things
(user mixing and matching incompatible software).


Ok, so how about keeping with this simple mechanism at the moment ?

If this versionning system is too limited in the future, I think we will 
need a global rework on the protocol, anyway.



--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 2/3] docs: update ivshmem device spec

2014-09-08 Thread David Marchand
Add some notes on the parts needed to use ivshmem devices: more specifically,
explain the purpose of an ivshmem server and the basic concept to use the
ivshmem devices in guests.
Move some parts of the documentation and re-organise it.

Signed-off-by: David Marchand david.march...@6wind.com
Reviewed-by: Claudio Fontana claudio.font...@huawei.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
---
 docs/specs/ivshmem_device_spec.txt |  124 +++-
 1 file changed, 93 insertions(+), 31 deletions(-)

diff --git a/docs/specs/ivshmem_device_spec.txt 
b/docs/specs/ivshmem_device_spec.txt
index 667a862..12f338e 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -2,30 +2,103 @@
 Device Specification for Inter-VM shared memory device
 --
 
-The Inter-VM shared memory device is designed to share a region of memory to
-userspace in multiple virtual guests.  The memory region does not belong to any
-guest, but is a POSIX memory object on the host.  Optionally, the device may
-support sending interrupts to other guests sharing the same memory region.
+The Inter-VM shared memory device is designed to share a memory region (created
+on the host via the POSIX shared memory API) between multiple QEMU processes
+running different guests. In order for all guests to be able to pick up the
+shared memory area, it is modeled by QEMU as a PCI device exposing said memory
+to the guest as a PCI BAR.
+The memory region does not belong to any guest, but is a POSIX memory object on
+the host. The host can access this shared memory if needed.
+
+The device also provides an optional communication mechanism between guests
+sharing the same memory object. More details about that in the section 'Guest 
to
+guest communication' section.
 
 
 The Inter-VM PCI device
 ---
 
-*BARs*
+From the VM point of view, the ivshmem PCI device supports three BARs.
+
+- BAR0 is a 1 Kbyte MMIO region to support registers and interrupts when MSI is
+  not used.
+- BAR1 is used for MSI-X when it is enabled in the device.
+- BAR2 is used to access the shared memory object.
+
+It is your choice how to use the device but you must choose between two
+behaviors :
+
+- basically, if you only need the shared memory part, you will map BAR2.
+  This way, you have access to the shared memory in guest and can use it as you
+  see fit (memnic, for example, uses it in userland
+  http://dpdk.org/browse/memnic).
+
+- BAR0 and BAR1 are used to implement an optional communication mechanism
+  through interrupts in the guests. If you need an event mechanism between the
+  guests accessing the shared memory, you will most likely want to write a
+  kernel driver that will handle interrupts. See details in the section 'Guest
+  to guest communication' section.
+
+The behavior is chosen when starting your QEMU processes:
+- no communication mechanism needed, the first QEMU to start creates the shared
+  memory on the host, subsequent QEMU processes will use it.
+
+- communication mechanism needed, an ivshmem server must be started before any
+  QEMU processes, then each QEMU process connects to the server unix socket.
+
+For more details on the QEMU ivshmem parameters, see qemu-doc documentation.
+
+
+Guest to guest communication
+
+
+This section details the communication mechanism between the guests accessing
+the ivhsmem shared memory.
 
-The device supports three BARs.  BAR0 is a 1 Kbyte MMIO region to support
-registers.  BAR1 is used for MSI-X when it is enabled in the device.  BAR2 is
-used to map the shared memory object from the host.  The size of BAR2 is
-specified when the guest is started and must be a power of 2 in size.
+*ivshmem server*
 
-*Registers*
+This server code is available in qemu.git/contrib/ivshmem-server.
 
-The device currently supports 4 registers of 32-bits each.  Registers
-are used for synchronization between guests sharing the same memory object when
-interrupts are supported (this requires using the shared memory server).
+The server must be started on the host before any guest.
+It creates a shared memory object then waits for clients to connect on a unix
+socket.
 
-The server assigns each VM an ID number and sends this ID number to the QEMU
-process when the guest starts.
+For each client (QEMU process) that connects to the server:
+- 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,
+- the server creates a new set of host eventfds associated to the new client 
and
+  sends this set to all already connected clients,
+- finally, the server sends all the eventfds sets for all clients to the new
+  client.
+
+The server signals all clients when one of them disconnects.
+
+The client IDs are limited to 16 bits because of the current implementation 
(see
+Doorbell

[PATCH v6 0/3] ivshmem: update documentation, add client/server tools

2014-09-08 Thread David Marchand
Here is a patchset containing an update on ivshmem specs documentation and
importing ivshmem server and client tools.
These tools have been written from scratch and are not related to what is
available in nahanni repository.
I put them in contrib/ directory as the qemu-doc.texi was already telling the
server was supposed to be there.

Changes since v5:
- 1st patch
* fixed remaining coding style issues
* fixed checks on snprintf return code
* prefixed all symbols to avoid pollution
* fixed spelling mistakes in comments
* fixed function descriptions in headers
* added a signal handler on SIGTERM and only unlink server socket when server 
exits
* fixed fd leaks I identified in client when reconnecting to a server
- 3rd patch
* remove wrong debug message
* do not delay pci bar2 register

Changes since v4:
- squashed patches 3-13 from v4 into first patch
- reused reported error when parsing arguments in server
- fixed spelling mistakes in documentation in second patch

Changes since v3:
- first patch is untouched
- just restored the Reviewed-By Claudio in second patch
- following patches 3-8 take into account Stefan's comments
- patches 9-12 take into account Gonglei's comments
- patch 13 adjusts ivshmem-server default values
- last patch introduces a change in the ivshmem client-server protocol to
  check a protocol version at connect time

Changes since v2:
- fixed license issues in ivshmem client/server (I took hw/virtio/virtio-rng.c
  file as a reference).

Changes since v1:
- moved client/server import patch before doc update,
- tried to re-organise the ivshmem_device_spec.txt file based on Claudio
  comments (still not sure if the result is that great, comments welcome),
- incorporated comments from Claudio, Eric and Cam,
- added more details on the server - client messages exchange (but sorry, no
  ASCII art here).

By the way, there are still some functionnalities that need description (use of
ioeventfd, the lack of irqfd support) and some parts of the ivshmem code clearly
need cleanup. I will try to address this in future patches when these first
patches are ok.


-- 
David Marchand

David Marchand (3):
  contrib: add ivshmem client and server
  docs: update ivshmem device spec
  ivshmem: add check on protocol version in QEMU

 Makefile|8 +
 configure   |3 +
 contrib/ivshmem-client/ivshmem-client.c |  440 +++
 contrib/ivshmem-client/ivshmem-client.h |  213 +++
 contrib/ivshmem-client/main.c   |  239 +
 contrib/ivshmem-server/ivshmem-server.c |  426 ++
 contrib/ivshmem-server/ivshmem-server.h |  165 
 contrib/ivshmem-server/main.c   |  264 +++
 docs/specs/ivshmem_device_spec.txt  |  127 ++---
 hw/misc/ivshmem.c   |   27 +-
 include/hw/misc/ivshmem.h   |   17 ++
 qemu-doc.texi   |   10 +-
 12 files changed, 1903 insertions(+), 36 deletions(-)
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c
 create mode 100644 include/hw/misc/ivshmem.h

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 1/3] contrib: add ivshmem client and server

2014-09-08 Thread David Marchand
When using ivshmem devices, notifications between guests can be sent as
interrupts using a ivshmem-server (typical use described in documentation).
The client is provided as a debug tool.

Signed-off-by: Olivier Matz olivier.m...@6wind.com
Signed-off-by: David Marchand david.march...@6wind.com
---
 Makefile|8 +
 configure   |3 +
 contrib/ivshmem-client/ivshmem-client.c |  433 +++
 contrib/ivshmem-client/ivshmem-client.h |  212 +++
 contrib/ivshmem-client/main.c   |  239 +
 contrib/ivshmem-server/ivshmem-server.c |  417 +
 contrib/ivshmem-server/ivshmem-server.h |  164 
 contrib/ivshmem-server/main.c   |  264 +++
 qemu-doc.texi   |   10 +-
 9 files changed, 1747 insertions(+), 3 deletions(-)
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

diff --git a/Makefile b/Makefile
index b33aaac..0575898 100644
--- a/Makefile
+++ b/Makefile
@@ -283,6 +283,14 @@ $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
$(call LINK, $^)
 
+IVSHMEM_CLIENT_OBJS=$(addprefix $(SRC_PATH)/contrib/ivshmem-client/, 
ivshmem-client.o main.o)
+ivshmem-client$(EXESUF): $(IVSHMEM_CLIENT_OBJS)
+   $(call LINK, $^)
+
+IVSHMEM_SERVER_OBJS=$(addprefix $(SRC_PATH)/contrib/ivshmem-server/, 
ivshmem-server.o main.o)
+ivshmem-server$(EXESUF): $(IVSHMEM_SERVER_OBJS) libqemuutil.a libqemustub.a
+   $(call LINK, $^)
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
diff --git a/configure b/configure
index 961bf6f..a41a16c 100755
--- a/configure
+++ b/configure
@@ -4125,6 +4125,9 @@ if test $want_tools = yes ; then
   if [ $linux = yes -o $bsd = yes -o $solaris = yes ] ; then
 tools=qemu-nbd\$(EXESUF) $tools
   fi
+  if [ $kvm = yes ] ; then
+tools=ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools
+  fi
 fi
 if test $softmmu = yes ; then
   if test $virtfs != no ; then
diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
new file mode 100644
index 000..11c805c
--- /dev/null
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -0,0 +1,433 @@
+/*
+ * Copyright 6WIND S.A., 2014
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include sys/types.h
+#include sys/socket.h
+#include sys/un.h
+
+#include qemu-common.h
+#include qemu/queue.h
+
+#include ivshmem-client.h
+
+/* log a message on stdout if verbose=1 */
+#define IVSHMEM_CLIENT_DEBUG(client, fmt, ...) do { \
+if ((client)-verbose) { \
+printf(fmt, ## __VA_ARGS__); \
+}\
+} while (0)
+
+/* read message from the unix socket */
+static int
+ivshmem_client_read_one_msg(IvshmemClient *client, long *index, int *fd)
+{
+int ret;
+struct msghdr msg;
+struct iovec iov[1];
+union {
+struct cmsghdr cmsg;
+char control[CMSG_SPACE(sizeof(int))];
+} msg_control;
+struct cmsghdr *cmsg;
+
+iov[0].iov_base = index;
+iov[0].iov_len = sizeof(*index);
+
+memset(msg, 0, sizeof(msg));
+msg.msg_iov = iov;
+msg.msg_iovlen = 1;
+msg.msg_control = msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+ret = recvmsg(client-sock_fd, msg, 0);
+if (ret  0) {
+IVSHMEM_CLIENT_DEBUG(client, cannot read message: %s\n,
+ strerror(errno));
+return -1;
+}
+if (ret == 0) {
+IVSHMEM_CLIENT_DEBUG(client, lost connection to server\n);
+return -1;
+}
+
+*fd = -1;
+
+for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+
+if (cmsg-cmsg_len != CMSG_LEN(sizeof(int)) ||
+cmsg-cmsg_level != SOL_SOCKET ||
+cmsg-cmsg_type != SCM_RIGHTS) {
+continue;
+}
+
+memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd));
+}
+
+return 0;
+}
+
+/* free a peer when the server advertises a disconnection or when the
+ * client is freed */
+static void
+ivshmem_client_free_peer(IvshmemClient *client, IvshmemClientPeer *peer)
+{
+unsigned vector;
+
+QTAILQ_REMOVE(client-peer_list, peer, next);
+for (vector = 0; vector  peer-vectors_count; vector++) {
+close(peer-vectors[vector]);
+}
+
+g_free(peer);
+}
+
+/* handle message coming from server (new peer, new vectors

[PATCH v6 3/3] ivshmem: add check on protocol version in QEMU

2014-09-08 Thread David Marchand
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
---
 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   |   27 +--
 include/hw/misc/ivshmem.h   |   17 +
 7 files changed, 69 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 11c805c..4413e26 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -201,10 +201,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);
@@ -216,7 +223,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 91dc208..ea290f6 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -99,6 +99,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 f85dcd2..38df7bb 100644
--- a/contrib/ivshmem-server/ivshmem-server.h
+++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -30,6 +30,7 @@
 #include sys/select.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

Re: [PATCH v5 1/3] contrib: add ivshmem client and server

2014-09-05 Thread David Marchand

Hello Michael,

On 09/04/2014 05:56 PM, Michael S. Tsirkin wrote:

+/* create the unix listening socket */
+sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
+if (sock_fd  0) {
+debug_log(server, cannot create socket: %s\n, strerror(errno));
+goto err_close_shm;
+}
+
+sun.sun_family = AF_UNIX;
+snprintf(sun.sun_path, sizeof(sun.sun_path), %s, server-unix_sock_path);
+unlink(sun.sun_path);


why unlink it?


Yes, this is wrong, because this means that when starting multiple
servers on the same socket, the last server is the one who wins ...
while I think it should be the opposite (first server wins, as it may
have some connected clients).

I have been scratching my head about this: when should I unlink ?

My current fix unlinks from ivshmem_server_close() (which should be the
right place).
I need to call this when exiting, but I can only do this when the server
exits gracefully (when an error occurs on the server socket or when 
receiving a SIGTERM).


If something unexpected happens (like a bug/crash or a SIGKILL), the
socket won't be unlinked and the next server process will refuse to start.
Is this something acceptable ?

Do you have a better idea ?


Thanks.

--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 3/3] ivshmem: add check on protocol version in QEMU

2014-09-05 Thread David Marchand

Hello Stefan,

On 09/05/2014 12:29 PM, Stefan Hajnoczi wrote:

On Thu, Sep 04, 2014 at 02:51:01PM +0200, David Marchand wrote:

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index ad210c8..0c4e016 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -184,10 +184,18 @@ ivshmem_client_connect(IvshmemClient *client)
  goto err_close;
  }

-/* first, we expect our index + a fd == -1 */
+/* first, we expect a protocol version */
+if (read_one_msg(client, tmp, fd)  0 ||
+(tmp != IVSHMEM_PROTOCOL_VERSION) || fd != -1) {
+debug_log(client, cannot read from server\n);
+goto err_close;
+}
+debug_log(client, our_id=%ld\n, client-local.id);


This debug_log() is probably not intentional.  local.id will always be
-1 here so the output is not useful.


Yes, this is most likely a merge/rebase issue.
Will remove this.




+static void ivshmem_check_version(void *opaque, const uint8_t * buf, int flags)
+{
+IVShmemState *s = opaque;
+PCIDevice *dev = PCI_DEVICE(s);
+int tmp;
+long version;
+
+memcpy(version, buf, sizeof(long));
+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 
ivhsmem-


Hum, typo: ivhs - ivsh.


+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, finish init and switch to real chardev 
+handler\n);
+
+pci_register_bar(dev, 2, s-ivshmem_attr, s-bar);


Not sure if it is okay to delay PCI initialization to a fd hander
callback.

If the version message is too slow the guest could see the PCI adapter
without the BAR!

Did you move this code in order to prevent the guest from accessing the
device before it has connected to the server?  Perhaps the device needs
a state field that tracks whether or not it is ready for operation.  Any
access before RUNNING state is reached will be ignored (?).


Yes, exactly.

There already is a synchronisation mechanism described in the documentation:
When using the server, since the server is a separate process, the VM 
ID will only be set when the device is ready (shared memory is received 
from the server and accessible via the device).  If the device is not 
ready, the IVPosition will return -1.
Applications should ensure that they have a valid VM ID before accessing 
the shared memory.


So actually, this move is unneeded if ivshmem users comply to this.

I will let the init stuff (pci_register_bar + gmalloc) where it was 
before, ivshmem_check_version will only switch the chardev handler.


What do you think about this ?


Thanks.

--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 0/3] ivshmem: update documentation, add client/server tools

2014-09-04 Thread David Marchand
Here is a patchset containing an update on ivshmem specs documentation and
importing ivshmem server and client tools.
These tools have been written from scratch and are not related to what is
available in nahanni repository.
I put them in contrib/ directory as the qemu-doc.texi was already telling the
server was supposed to be there.

Changes since v4:
- squashed patches 3-13 from v4 into first patch
- reused reported error when parsing arguments in server
- fixed spelling mistakes in documentation in second patch

Changes since v3:
- first patch is untouched
- just restored the Reviewed-By Claudio in second patch
- following patches 3-8 take into account Stefan's comments
- patches 9-12 take into account Gonglei's comments
- patch 13 adjusts ivshmem-server default values
- last patch introduces a change in the ivshmem client-server protocol to
  check a protocol version at connect time

Changes since v2:
- fixed license issues in ivshmem client/server (I took hw/virtio/virtio-rng.c
  file as a reference).

Changes since v1:
- moved client/server import patch before doc update,
- tried to re-organise the ivshmem_device_spec.txt file based on Claudio
  comments (still not sure if the result is that great, comments welcome),
- incorporated comments from Claudio, Eric and Cam,
- added more details on the server - client messages exchange (but sorry, no
  ASCII art here).

By the way, there are still some functionnalities that need description (use of
ioeventfd, the lack of irqfd support) and some parts of the ivshmem code clearly
need cleanup. I will try to address this in future patches when these first
patches are ok.


-- 
David Marchand

David Marchand (3):
  contrib: add ivshmem client and server
  docs: update ivshmem device spec
  ivshmem: add check on protocol version in QEMU

 Makefile|8 +
 configure   |3 +
 contrib/ivshmem-client/ivshmem-client.c |  413 +++
 contrib/ivshmem-client/ivshmem-client.h |  240 ++
 contrib/ivshmem-client/main.c   |  237 ++
 contrib/ivshmem-server/ivshmem-server.c |  402 ++
 contrib/ivshmem-server/ivshmem-server.h |  187 ++
 contrib/ivshmem-server/main.c   |  244 ++
 docs/specs/ivshmem_device_spec.txt  |  127 +++---
 hw/misc/ivshmem.c   |   43 +++-
 include/hw/misc/ivshmem.h   |   17 ++
 qemu-doc.texi   |   10 +-
 12 files changed, 1888 insertions(+), 43 deletions(-)
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c
 create mode 100644 include/hw/misc/ivshmem.h

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 3/3] ivshmem: add check on protocol version in QEMU

2014-09-04 Thread David Marchand
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
---
 contrib/ivshmem-client/ivshmem-client.c |   14 +++---
 contrib/ivshmem-client/ivshmem-client.h |1 +
 contrib/ivshmem-server/ivshmem-server.c |7 +
 contrib/ivshmem-server/ivshmem-server.h |1 +
 docs/specs/ivshmem_device_spec.txt  |9 ---
 hw/misc/ivshmem.c   |   43 ---
 include/hw/misc/ivshmem.h   |   17 
 7 files changed, 77 insertions(+), 15 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 ad210c8..0c4e016 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -184,10 +184,18 @@ ivshmem_client_connect(IvshmemClient *client)
 goto err_close;
 }
 
-/* first, we expect our index + a fd == -1 */
+/* first, we expect a protocol version */
+if (read_one_msg(client, tmp, fd)  0 ||
+(tmp != IVSHMEM_PROTOCOL_VERSION) || fd != -1) {
+debug_log(client, cannot read from server\n);
+goto err_close;
+}
+debug_log(client, our_id=%ld\n, client-local.id);
+
+/* then, we expect our index + a fd == -1 */
 if (read_one_msg(client, client-local.id, fd)  0 ||
 client-local.id  0 || fd != -1) {
-debug_log(client, cannot read from server\n);
+debug_log(client, cannot read from server (2)\n);
 goto err_close;
 }
 debug_log(client, our_id=%ld\n, client-local.id);
@@ -196,7 +204,7 @@ ivshmem_client_connect(IvshmemClient *client)
  * is not used */
 if (read_one_msg(client, tmp, fd)  0 ||
 tmp != -1 || fd  0) {
-debug_log(client, cannot read from server (2)\n);
+debug_log(client, cannot read from server (3)\n);
 goto err_close;
 }
 debug_log(client, shm_fd=%d\n, fd);
diff --git a/contrib/ivshmem-client/ivshmem-client.h 
b/contrib/ivshmem-client/ivshmem-client.h
index 45f2b64..8d6ab35 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 f441da7..670c58c 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -99,6 +99,13 @@ send_initial_info(IvshmemServer *server, IvshmemServerPeer 
*peer)
 {
 int ret;
 
+/* send our protool version first */
+ret = send_one_msg(peer-sock_fd, IVSHMEM_PROTOCOL_VERSION, -1);
+if (ret  0) {
+debug_log(server, cannot send version: %s\n, strerror(errno));
+return -1;
+}
+
 /* send the peer id to the client */
 ret = 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 5ccc7af..e76e4fe 100644
--- a/contrib/ivshmem-server/ivshmem-server.h
+++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -30,6 +30,7 @@
 #include sys/select.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

[PATCH v5 1/3] contrib: add ivshmem client and server

2014-09-04 Thread David Marchand
When using ivshmem devices, notifications between guests can be sent as
interrupts using a ivshmem-server (typical use described in documentation).
The client is provided as a debug tool.

Signed-off-by: Olivier Matz olivier.m...@6wind.com
Signed-off-by: David Marchand david.march...@6wind.com
---
 Makefile|8 +
 configure   |3 +
 contrib/ivshmem-client/ivshmem-client.c |  405 +++
 contrib/ivshmem-client/ivshmem-client.h |  239 ++
 contrib/ivshmem-client/main.c   |  237 ++
 contrib/ivshmem-server/ivshmem-server.c |  395 ++
 contrib/ivshmem-server/ivshmem-server.h |  186 ++
 contrib/ivshmem-server/main.c   |  244 +++
 qemu-doc.texi   |   10 +-
 9 files changed, 1724 insertions(+), 3 deletions(-)
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

diff --git a/Makefile b/Makefile
index b33aaac..0575898 100644
--- a/Makefile
+++ b/Makefile
@@ -283,6 +283,14 @@ $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
$(call LINK, $^)
 
+IVSHMEM_CLIENT_OBJS=$(addprefix $(SRC_PATH)/contrib/ivshmem-client/, 
ivshmem-client.o main.o)
+ivshmem-client$(EXESUF): $(IVSHMEM_CLIENT_OBJS)
+   $(call LINK, $^)
+
+IVSHMEM_SERVER_OBJS=$(addprefix $(SRC_PATH)/contrib/ivshmem-server/, 
ivshmem-server.o main.o)
+ivshmem-server$(EXESUF): $(IVSHMEM_SERVER_OBJS) libqemuutil.a libqemustub.a
+   $(call LINK, $^)
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
diff --git a/configure b/configure
index 961bf6f..a41a16c 100755
--- a/configure
+++ b/configure
@@ -4125,6 +4125,9 @@ if test $want_tools = yes ; then
   if [ $linux = yes -o $bsd = yes -o $solaris = yes ] ; then
 tools=qemu-nbd\$(EXESUF) $tools
   fi
+  if [ $kvm = yes ] ; then
+tools=ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools
+  fi
 fi
 if test $softmmu = yes ; then
   if test $virtfs != no ; then
diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
new file mode 100644
index 000..ad210c8
--- /dev/null
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -0,0 +1,405 @@
+/*
+ * Copyright 6WIND S.A., 2014
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include sys/types.h
+#include sys/socket.h
+#include sys/un.h
+
+#include qemu-common.h
+#include qemu/queue.h
+
+#include ivshmem-client.h
+
+/* log a message on stdout if verbose=1 */
+#define debug_log(client, fmt, ...) do { \
+if ((client)-verbose) { \
+printf(fmt, ## __VA_ARGS__); \
+}\
+} while (0)
+
+/* read message from the unix socket */
+static int
+read_one_msg(IvshmemClient *client, long *index, int *fd)
+{
+int ret;
+struct msghdr msg;
+struct iovec iov[1];
+union {
+struct cmsghdr cmsg;
+char control[CMSG_SPACE(sizeof(int))];
+} msg_control;
+struct cmsghdr *cmsg;
+
+iov[0].iov_base = index;
+iov[0].iov_len = sizeof(*index);
+
+memset(msg, 0, sizeof(msg));
+msg.msg_iov = iov;
+msg.msg_iovlen = 1;
+msg.msg_control = msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+ret = recvmsg(client-sock_fd, msg, 0);
+if (ret  0) {
+debug_log(client, cannot read message: %s\n, strerror(errno));
+return -1;
+}
+if (ret == 0) {
+debug_log(client, lost connection to server\n);
+return -1;
+}
+
+*fd = -1;
+
+for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+
+if (cmsg-cmsg_len != CMSG_LEN(sizeof(int)) ||
+cmsg-cmsg_level != SOL_SOCKET ||
+cmsg-cmsg_type != SCM_RIGHTS) {
+continue;
+}
+
+memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd));
+}
+
+return 0;
+}
+
+/* free a peer when the server advertise a disconnection or when the
+ * client is freed */
+static void
+free_peer(IvshmemClient *client, IvshmemClientPeer *peer)
+{
+unsigned vector;
+
+QTAILQ_REMOVE(client-peer_list, peer, next);
+for (vector = 0; vector  peer-vectors_count; vector++) {
+close(peer-vectors[vector]);
+}
+
+g_free(peer);
+}
+
+/* handle message coming from server (new peer, new vectors) */
+static int
+handle_server_msg(IvshmemClient *client)
+{
+IvshmemClientPeer *peer

[PATCH v5 2/3] docs: update ivshmem device spec

2014-09-04 Thread David Marchand
Add some notes on the parts needed to use ivshmem devices: more specifically,
explain the purpose of an ivshmem server and the basic concept to use the
ivshmem devices in guests.
Move some parts of the documentation and re-organise it.

Signed-off-by: David Marchand david.march...@6wind.com
Reviewed-by: Claudio Fontana claudio.font...@huawei.com
---
 docs/specs/ivshmem_device_spec.txt |  124 +++-
 1 file changed, 93 insertions(+), 31 deletions(-)

diff --git a/docs/specs/ivshmem_device_spec.txt 
b/docs/specs/ivshmem_device_spec.txt
index 667a862..12f338e 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -2,30 +2,103 @@
 Device Specification for Inter-VM shared memory device
 --
 
-The Inter-VM shared memory device is designed to share a region of memory to
-userspace in multiple virtual guests.  The memory region does not belong to any
-guest, but is a POSIX memory object on the host.  Optionally, the device may
-support sending interrupts to other guests sharing the same memory region.
+The Inter-VM shared memory device is designed to share a memory region (created
+on the host via the POSIX shared memory API) between multiple QEMU processes
+running different guests. In order for all guests to be able to pick up the
+shared memory area, it is modeled by QEMU as a PCI device exposing said memory
+to the guest as a PCI BAR.
+The memory region does not belong to any guest, but is a POSIX memory object on
+the host. The host can access this shared memory if needed.
+
+The device also provides an optional communication mechanism between guests
+sharing the same memory object. More details about that in the section 'Guest 
to
+guest communication' section.
 
 
 The Inter-VM PCI device
 ---
 
-*BARs*
+From the VM point of view, the ivshmem PCI device supports three BARs.
+
+- BAR0 is a 1 Kbyte MMIO region to support registers and interrupts when MSI is
+  not used.
+- BAR1 is used for MSI-X when it is enabled in the device.
+- BAR2 is used to access the shared memory object.
+
+It is your choice how to use the device but you must choose between two
+behaviors :
+
+- basically, if you only need the shared memory part, you will map BAR2.
+  This way, you have access to the shared memory in guest and can use it as you
+  see fit (memnic, for example, uses it in userland
+  http://dpdk.org/browse/memnic).
+
+- BAR0 and BAR1 are used to implement an optional communication mechanism
+  through interrupts in the guests. If you need an event mechanism between the
+  guests accessing the shared memory, you will most likely want to write a
+  kernel driver that will handle interrupts. See details in the section 'Guest
+  to guest communication' section.
+
+The behavior is chosen when starting your QEMU processes:
+- no communication mechanism needed, the first QEMU to start creates the shared
+  memory on the host, subsequent QEMU processes will use it.
+
+- communication mechanism needed, an ivshmem server must be started before any
+  QEMU processes, then each QEMU process connects to the server unix socket.
+
+For more details on the QEMU ivshmem parameters, see qemu-doc documentation.
+
+
+Guest to guest communication
+
+
+This section details the communication mechanism between the guests accessing
+the ivhsmem shared memory.
 
-The device supports three BARs.  BAR0 is a 1 Kbyte MMIO region to support
-registers.  BAR1 is used for MSI-X when it is enabled in the device.  BAR2 is
-used to map the shared memory object from the host.  The size of BAR2 is
-specified when the guest is started and must be a power of 2 in size.
+*ivshmem server*
 
-*Registers*
+This server code is available in qemu.git/contrib/ivshmem-server.
 
-The device currently supports 4 registers of 32-bits each.  Registers
-are used for synchronization between guests sharing the same memory object when
-interrupts are supported (this requires using the shared memory server).
+The server must be started on the host before any guest.
+It creates a shared memory object then waits for clients to connect on a unix
+socket.
 
-The server assigns each VM an ID number and sends this ID number to the QEMU
-process when the guest starts.
+For each client (QEMU process) that connects to the server:
+- 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,
+- the server creates a new set of host eventfds associated to the new client 
and
+  sends this set to all already connected clients,
+- finally, the server sends all the eventfds sets for all clients to the new
+  client.
+
+The server signals all clients when one of them disconnects.
+
+The client IDs are limited to 16 bits because of the current implementation 
(see
+Doorbell register in 'PCI device registers' subsection). Hence

Re: [PATCH v4 00/14] ivshmem: update documentation, add client/server tools

2014-09-03 Thread David Marchand

Hello Eric,

On 09/02/2014 10:31 PM, Eric Blake wrote:

On 09/02/2014 09:25 AM, David Marchand wrote:

Here is a patchset containing an update on ivshmem specs documentation and
importing ivshmem server and client tools.
These tools have been written from scratch and are not related to what is
available in nahanni repository.
I put them in contrib/ directory as the qemu-doc.texi was already telling the
server was supposed to be there.

Changes since v3:
- first patch is untouched
- just restored the Reviewed-By Claudio in second patch
- following patches 3-8 take into account Stefan's comments
- patches 9-12 take into account Gonglei's comments
- patch 13 adjusts ivshmem-server default values
- last patch introduces a change in the ivshmem client-server protocol to
   check a protocol version at connect time


Rather than introducing new files with bugs, followed by patches to
clean it up, why not just introduce the new files correct in the first
place?  I think you are better off squashing in a lot of the cleanup
patches into patch 1.


Actually, I mentioned this in a previous email but did not get any comment.
So, I preferred to send the splitted patches to ease review (from my 
point of view).


Once code looks fine enough, I intend to keep only three patches :
- one for the initial import of ivshmem-client / server
- one for the documentation update
- one last with the protocol change

Is it okay this way ?


--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 00/14] ivshmem: update documentation, add client/server tools

2014-09-02 Thread David Marchand
Here is a patchset containing an update on ivshmem specs documentation and
importing ivshmem server and client tools.
These tools have been written from scratch and are not related to what is
available in nahanni repository.
I put them in contrib/ directory as the qemu-doc.texi was already telling the
server was supposed to be there.

Changes since v3:
- first patch is untouched
- just restored the Reviewed-By Claudio in second patch
- following patches 3-8 take into account Stefan's comments
- patches 9-12 take into account Gonglei's comments
- patch 13 adjusts ivshmem-server default values
- last patch introduces a change in the ivshmem client-server protocol to
  check a protocol version at connect time

Changes since v2:
- fixed license issues in ivshmem client/server (I took hw/virtio/virtio-rng.c
  file as a reference).

Changes since v1:
- moved client/server import patch before doc update,
- tried to re-organise the ivshmem_device_spec.txt file based on Claudio
  comments (still not sure if the result is that great, comments welcome),
- incorporated comments from Claudio, Eric and Cam,
- added more details on the server - client messages exchange (but sorry, no
  ASCII art here).

By the way, there are still some functionnalities that need description (use of
ioeventfd, the lack of irqfd support) and some parts of the ivshmem code clearly
need cleanup. I will try to address this in future patches when these first
patches are ok.


-- 
David Marchand

David Marchand (14):
  contrib: add ivshmem client and server
  docs: update ivshmem device spec
  contrib/ivshmem-*: comply with QEMU coding style
  contrib/ivshmem-*: reuse qemu/queue.h
  contrib/ivshmem-*: switch to QEMU headers
  contrib/ivshmem-server: set client sockets as non blocking
  contrib/ivshmem-*: add missing const and static attrs
  contrib/ivshmem-*: plug client and server in QEMU top Makefile
  contrib/ivshmem-*: switch to g_malloc0/g_free
  contrib/ivshmem-server: fix mem leak on error
  contrib/ivshmem-*: rework error handling
  contrib/ivshmem-*: various fixes
  contrib/ivshmem-server: align server default parameter values
  ivshmem: add check on protocol version in QEMU

 Makefile|8 +
 configure   |3 +
 contrib/ivshmem-client/ivshmem-client.c |  413 +++
 contrib/ivshmem-client/ivshmem-client.h |  240 ++
 contrib/ivshmem-client/main.c   |  237 ++
 contrib/ivshmem-server/ivshmem-server.c |  402 ++
 contrib/ivshmem-server/ivshmem-server.h |  187 ++
 contrib/ivshmem-server/main.c   |  243 ++
 docs/specs/ivshmem_device_spec.txt  |  127 +++---
 hw/misc/ivshmem.c   |   43 +++-
 include/hw/misc/ivshmem.h   |   17 ++
 qemu-doc.texi   |   10 +-
 12 files changed, 1887 insertions(+), 43 deletions(-)
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c
 create mode 100644 include/hw/misc/ivshmem.h

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 06/14] contrib/ivshmem-server: set client sockets as non blocking

2014-09-02 Thread David Marchand
Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-server/ivshmem-server.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index 0afa6e8..e0d4d1d 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -128,7 +128,8 @@ handle_new_conn(IvshmemServer *server)
 
 /* accept the incoming connection */
 unaddr_len = sizeof(unaddr);
-newfd = accept(server-sock_fd, (struct sockaddr *)unaddr, unaddr_len);
+newfd = accept4(server-sock_fd, (struct sockaddr *)unaddr, unaddr_len,
+SOCK_NONBLOCK);
 if (newfd  0) {
 debug_log(server, cannot accept() %s\n, strerror(errno));
 return -1;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 04/14] contrib/ivshmem-*: reuse qemu/queue.h

2014-09-02 Thread David Marchand
Switch to qemu/queue.h strutures.

Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/ivshmem-client.c |   17 
 contrib/ivshmem-client/ivshmem-client.h |7 ---
 contrib/ivshmem-server/ivshmem-server.c |   33 ---
 contrib/ivshmem-server/ivshmem-server.h |7 ---
 4 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index 3f6ca98..ce3a5d2 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -14,12 +14,13 @@
 #include signal.h
 #include unistd.h
 #include inttypes.h
-#include sys/queue.h
 
 #include sys/types.h
 #include sys/socket.h
 #include sys/un.h
 
+#include qemu/queue.h
+
 #include ivshmem-client.h
 
 /* log a message on stdout if verbose=1 */
@@ -84,7 +85,7 @@ free_peer(IvshmemClient *client, IvshmemClientPeer *peer)
 {
 unsigned vector;
 
-TAILQ_REMOVE(client-peer_list, peer, next);
+QTAILQ_REMOVE(client-peer_list, peer, next);
 for (vector = 0; vector  peer-vectors_count; vector++) {
 close(peer-vectors[vector]);
 }
@@ -131,7 +132,7 @@ handle_server_msg(IvshmemClient *client)
 memset(peer, 0, sizeof(*peer));
 peer-id = peer_id;
 peer-vectors_count = 0;
-TAILQ_INSERT_TAIL(client-peer_list, peer, next);
+QTAILQ_INSERT_TAIL(client-peer_list, peer, next);
 debug_log(client, new peer id = %ld\n, peer_id);
 }
 
@@ -161,7 +162,7 @@ ivshmem_client_init(IvshmemClient *client, const char 
*unix_sock_path,
 client-local.vectors[i] = -1;
 }
 
-TAILQ_INIT(client-peer_list);
+QTAILQ_INIT(client-peer_list);
 client-local.id = -1;
 
 client-notif_cb = notif_cb;
@@ -230,7 +231,7 @@ ivshmem_client_close(IvshmemClient *client)
 
 debug_log(client, close client\n);
 
-while ((peer = TAILQ_FIRST(client-peer_list)) != NULL) {
+while ((peer = QTAILQ_FIRST(client-peer_list)) != NULL) {
 free_peer(client, peer);
 }
 
@@ -363,7 +364,7 @@ ivshmem_client_notify_broadcast(const IvshmemClient *client)
 IvshmemClientPeer *peer;
 int ret = 0;
 
-TAILQ_FOREACH(peer, client-peer_list, next) {
+QTAILQ_FOREACH(peer, client-peer_list, next) {
 if (ivshmem_client_notify_all_vects(client, peer)  0) {
 ret = -1;
 }
@@ -382,7 +383,7 @@ ivshmem_client_search_peer(IvshmemClient *client, long 
peer_id)
 return client-local;
 }
 
-TAILQ_FOREACH(peer, client-peer_list, next) {
+QTAILQ_FOREACH(peer, client-peer_list, next) {
 if (peer-id == peer_id) {
 return peer;
 }
@@ -406,7 +407,7 @@ ivshmem_client_dump(const IvshmemClient *client)
 }
 
 /* dump peers */
-TAILQ_FOREACH(peer, client-peer_list, next) {
+QTAILQ_FOREACH(peer, client-peer_list, next) {
 printf(peer_id = %ld\n, peer-id);
 
 for (vector = 0; vector  peer-vectors_count; vector++) {
diff --git a/contrib/ivshmem-client/ivshmem-client.h 
b/contrib/ivshmem-client/ivshmem-client.h
index 0fe0c94..e3b284d 100644
--- a/contrib/ivshmem-client/ivshmem-client.h
+++ b/contrib/ivshmem-client/ivshmem-client.h
@@ -21,7 +21,8 @@
 
 #include limits.h
 #include sys/select.h
-#include sys/queue.h
+
+#include qemu/queue.h
 
 /**
  * Maximum number of notification vectors supported by the client
@@ -40,12 +41,12 @@
  * client in (IvshmemClient)-local.
  */
 typedef struct IvshmemClientPeer {
-TAILQ_ENTRY(IvshmemClientPeer) next; /** next in list*/
+QTAILQ_ENTRY(IvshmemClientPeer) next;/** next in list*/
 long id; /** the id of the peer */
 int vectors[IVSHMEM_CLIENT_MAX_VECTORS]; /** one fd per vector */
 unsigned vectors_count;  /** number of vectors */
 } IvshmemClientPeer;
-TAILQ_HEAD(IvshmemClientPeerList, IvshmemClientPeer);
+QTAILQ_HEAD(IvshmemClientPeerList, IvshmemClientPeer);
 
 typedef struct IvshmemClientPeerList IvshmemClientPeerList;
 typedef struct IvshmemClient IvshmemClient;
diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index 20fbac0..e58864d 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -16,7 +16,6 @@
 #include inttypes.h
 #include fcntl.h
 
-#include sys/queue.h
 #include sys/mman.h
 #include sys/stat.h
 #include sys/types.h
@@ -24,6 +23,8 @@
 #include sys/un.h
 #include sys/eventfd.h
 
+#include qemu/queue.h
+
 #include ivshmem-server.h
 
 /* log a message on stdout if verbose=1 */
@@ -33,14 +34,6 @@
 }\
 } while (0)
 
-/* browse the queue, allowing to remove/free the current element */
-#defineTAILQ_FOREACH_SAFE(var, var2, head, field)\
-for ((var) = TAILQ_FIRST((head)),\
- (var2) = ((var) ? TAILQ_NEXT((var), field) : NULL

[PATCH v4 10/14] contrib/ivshmem-server: fix mem leak on error

2014-09-02 Thread David Marchand
Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-server/ivshmem-server.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index 15d468c..4732dab 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -190,8 +190,8 @@ fail:
 while (i--) {
 close(peer-vectors[i]);
 }
-peer-sock_fd = -1;
 close(newfd);
+g_free(peer);
 return -1;
 }
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 09/14] contrib/ivshmem-*: switch to g_malloc0/g_free

2014-09-02 Thread David Marchand
Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/ivshmem-client.c |9 ++---
 contrib/ivshmem-server/ivshmem-server.c |   12 ++--
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index 2ba40a7..a08f4d9 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -82,7 +82,7 @@ free_peer(IvshmemClient *client, IvshmemClientPeer *peer)
 close(peer-vectors[vector]);
 }
 
-free(peer);
+g_free(peer);
 }
 
 /* handle message coming from server (new peer, new vectors) */
@@ -116,12 +116,7 @@ handle_server_msg(IvshmemClient *client)
 
 /* new peer */
 if (peer == NULL) {
-peer = malloc(sizeof(*peer));
-if (peer == NULL) {
-debug_log(client, cannot allocate new peer\n);
-return -1;
-}
-memset(peer, 0, sizeof(*peer));
+peer = g_malloc0(sizeof(*peer));
 peer-id = peer_id;
 peer-vectors_count = 0;
 QTAILQ_INSERT_TAIL(client-peer_list, peer, next);
diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index e0d4d1d..15d468c 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -90,7 +90,7 @@ free_peer(IvshmemServer *server, IvshmemServerPeer *peer)
 close(peer-vectors[vector]);
 }
 
-free(peer);
+g_free(peer);
 }
 
 /* send the peer id and the shm_fd just after a new client connection */
@@ -138,15 +138,7 @@ handle_new_conn(IvshmemServer *server)
 debug_log(server, accept()=%d\n, newfd);
 
 /* allocate new structure for this peer */
-peer = malloc(sizeof(*peer));
-if (peer == NULL) {
-debug_log(server, cannot allocate new peer\n);
-close(newfd);
-return -1;
-}
-
-/* initialize the peer struct, one eventfd per vector */
-memset(peer, 0, sizeof(*peer));
+peer = g_malloc0(sizeof(*peer));
 peer-sock_fd = newfd;
 
 /* get an unused peer id */
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 02/14] docs: update ivshmem device spec

2014-09-02 Thread David Marchand
Add some notes on the parts needed to use ivshmem devices: more specifically,
explain the purpose of an ivshmem server and the basic concept to use the
ivshmem devices in guests.
Move some parts of the documentation and re-organise it.

Signed-off-by: David Marchand david.march...@6wind.com
Reviewed-by: Claudio Fontana claudio.font...@huawei.com
---
 docs/specs/ivshmem_device_spec.txt |  124 +++-
 1 file changed, 93 insertions(+), 31 deletions(-)

diff --git a/docs/specs/ivshmem_device_spec.txt 
b/docs/specs/ivshmem_device_spec.txt
index 667a862..f5f2b95 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -2,30 +2,103 @@
 Device Specification for Inter-VM shared memory device
 --
 
-The Inter-VM shared memory device is designed to share a region of memory to
-userspace in multiple virtual guests.  The memory region does not belong to any
-guest, but is a POSIX memory object on the host.  Optionally, the device may
-support sending interrupts to other guests sharing the same memory region.
+The Inter-VM shared memory device is designed to share a memory region (created
+on the host via the POSIX shared memory API) between multiple QEMU processes
+running different guests. In order for all guests to be able to pick up the
+shared memory area, it is modeled by QEMU as a PCI device exposing said memory
+to the guest as a PCI BAR.
+The memory region does not belong to any guest, but is a POSIX memory object on
+the host. The host can access this shared memory if needed.
+
+The device also provides an optional communication mechanism between guests
+sharing the same memory object. More details about that in the section 'Guest 
to
+guest communication' section.
 
 
 The Inter-VM PCI device
 ---
 
-*BARs*
+From the VM point of view, the ivshmem PCI device supports three BARs.
+
+- BAR0 is a 1 Kbyte MMIO region to support registers and interrupts when MSI is
+  not used.
+- BAR1 is used for MSI-X when it is enabled in the device.
+- BAR2 is used to access the shared memory object.
+
+It is your choice how to use the device but you must choose between two
+behaviors :
+
+- basically, if you only need the shared memory part, you will map BAR2.
+  This way, you have access to the shared memory in guest and can use it as you
+  see fit (memnic, for example, uses it in userland
+  http://dpdk.org/browse/memnic).
+
+- BAR0 and BAR1 are used to implement an optional communication mechanism
+  through interrupts in the guests. If you need an event mechanism between the
+  guests accessing the shared memory, you will most likely want to write a
+  kernel driver that will handle interrupts. See details in the section 'Guest
+  to guest communication' section.
+
+The behavior is chosen when starting your QEMU processes:
+- no communication mechanism needed, the first QEMU to start creates the shared
+  memory on the host, subsequent QEMU processes will use it.
+
+- communication mechanism needed, an ivshmem server must be started before any
+  QEMU processes, then each QEMU process connects to the server unix socket.
+
+For more details on the QEMU ivshmem parameters, see qemu-doc documentation.
+
+
+Guest to guest communication
+
+
+This section details the communication mechanism between the guests accessing
+the ivhsmem shared memory.
 
-The device supports three BARs.  BAR0 is a 1 Kbyte MMIO region to support
-registers.  BAR1 is used for MSI-X when it is enabled in the device.  BAR2 is
-used to map the shared memory object from the host.  The size of BAR2 is
-specified when the guest is started and must be a power of 2 in size.
+*ivshmem server*
 
-*Registers*
+This server code is available in qemu.git/contrib/ivshmem-server.
 
-The device currently supports 4 registers of 32-bits each.  Registers
-are used for synchronization between guests sharing the same memory object when
-interrupts are supported (this requires using the shared memory server).
+The server must be started on the host before any guest.
+It creates a shared memory object then waits for clients to connect on an unix
+socket.
 
-The server assigns each VM an ID number and sends this ID number to the QEMU
-process when the guest starts.
+For each client (QEMU processes) that connects to the server:
+- 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,
+- the server creates a new set of host eventfds associated to the new client 
and
+  sends this set to all already connected clients,
+- finally, the server sends all the eventfds sets for all clients to the new
+  client.
+
+The server signals all clients when one of them disconnects.
+
+The client IDs are limited to 16 bits because of the current implementation 
(see
+Doorbell register in 'PCI device registers' subsection

[PATCH v4 11/14] contrib/ivshmem-*: rework error handling

2014-09-02 Thread David Marchand
Following Gonglei comments, rework error handling using goto.

Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/ivshmem-client.c |   17 -
 contrib/ivshmem-server/ivshmem-server.c |   19 ++-
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index a08f4d9..e9a19ff 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -180,18 +180,14 @@ ivshmem_client_connect(IvshmemClient *client)
 if (connect(client-sock_fd, (struct sockaddr *)sun, sizeof(sun))  0) {
 debug_log(client, cannot connect to %s: %s\n, sun.sun_path,
   strerror(errno));
-close(client-sock_fd);
-client-sock_fd = -1;
-return -1;
+goto err_close;
 }
 
 /* first, we expect our index + a fd == -1 */
 if (read_one_msg(client, client-local.id, fd)  0 ||
 client-local.id  0 || fd != -1) {
 debug_log(client, cannot read from server\n);
-close(client-sock_fd);
-client-sock_fd = -1;
-return -1;
+goto err_close;
 }
 debug_log(client, our_id=%ld\n, client-local.id);
 
@@ -200,13 +196,16 @@ ivshmem_client_connect(IvshmemClient *client)
 if (read_one_msg(client, tmp, fd)  0 ||
 tmp != -1 || fd  0) {
 debug_log(client, cannot read from server (2)\n);
-close(client-sock_fd);
-client-sock_fd = -1;
-return -1;
+goto err_close;
 }
 debug_log(client, shm_fd=%d\n, fd);
 
 return 0;
+
+err_close:
+close(client-sock_fd);
+client-sock_fd = -1;
+return -1;
 }
 
 /* close connection to the server, and free all peer structures */
diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index 4732dab..f441da7 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -264,7 +264,7 @@ ivshmem_server_start(IvshmemServer *server)
 if (ivshmem_ftruncate(shm_fd, server-shm_size)  0) {
 fprintf(stderr, ftruncate(%s) failed: %s\n, server-shm_path,
 strerror(errno));
-return -1;
+goto err_close_shm;
 }
 
 debug_log(server, create  bind socket %s\n, server-unix_sock_path);
@@ -273,8 +273,7 @@ ivshmem_server_start(IvshmemServer *server)
 sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
 if (sock_fd  0) {
 debug_log(server, cannot create socket: %s\n, strerror(errno));
-close(shm_fd);
-return -1;
+goto err_close_shm;
 }
 
 sun.sun_family = AF_UNIX;
@@ -283,22 +282,24 @@ ivshmem_server_start(IvshmemServer *server)
 if (bind(sock_fd, (struct sockaddr *)sun, sizeof(sun))  0) {
 debug_log(server, cannot connect to %s: %s\n, sun.sun_path,
   strerror(errno));
-close(sock_fd);
-close(shm_fd);
-return -1;
+goto err_close_sock;
 }
 
 if (listen(sock_fd, IVSHMEM_SERVER_LISTEN_BACKLOG)  0) {
 debug_log(server, listen() failed: %s\n, strerror(errno));
-close(sock_fd);
-close(shm_fd);
-return -1;
+goto err_close_sock;
 }
 
 server-sock_fd = sock_fd;
 server-shm_fd = shm_fd;
 
 return 0;
+
+err_close_sock:
+close(sock_fd);
+err_close_shm:
+close(shm_fd);
+return -1;
 }
 
 /* close connections to clients, the unix socket and the shm fd */
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 12/14] contrib/ivshmem-*: various fixes

2014-09-02 Thread David Marchand
More fixes following Gonglei comments:
- add a missing \n in a debug message.
- add an explicit initialisation of sock_fd.
- fix a check on vector index.

Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/ivshmem-client.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index e9a19ff..ad210c8 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -105,7 +105,7 @@ handle_server_msg(IvshmemClient *client)
 if (fd == -1) {
 
 if (peer == NULL || peer == client-local) {
-debug_log(client, receive delete for invalid peer %ld, peer_id);
+debug_log(client, receive delete for invalid peer %ld\n, 
peer_id);
 return -1;
 }
 
@@ -155,6 +155,7 @@ ivshmem_client_init(IvshmemClient *client, const char 
*unix_sock_path,
 client-notif_cb = notif_cb;
 client-notif_arg = notif_arg;
 client-verbose = verbose;
+client-sock_fd = -1;
 
 return 0;
 }
@@ -309,7 +310,7 @@ ivshmem_client_notify(const IvshmemClient *client,
 uint64_t kick;
 int fd;
 
-if (vector  peer-vectors_count) {
+if (vector = peer-vectors_count) {
 debug_log(client, invalid vector %u on peer %ld\n, vector, peer-id);
 return -1;
 }
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 14/14] ivshmem: add check on protocol version in QEMU

2014-09-02 Thread David Marchand
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
---
 contrib/ivshmem-client/ivshmem-client.c |   14 +++---
 contrib/ivshmem-client/ivshmem-client.h |1 +
 contrib/ivshmem-server/ivshmem-server.c |7 +
 contrib/ivshmem-server/ivshmem-server.h |1 +
 docs/specs/ivshmem_device_spec.txt  |9 ---
 hw/misc/ivshmem.c   |   43 ---
 include/hw/misc/ivshmem.h   |   17 
 7 files changed, 77 insertions(+), 15 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 ad210c8..0c4e016 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -184,10 +184,18 @@ ivshmem_client_connect(IvshmemClient *client)
 goto err_close;
 }
 
-/* first, we expect our index + a fd == -1 */
+/* first, we expect a protocol version */
+if (read_one_msg(client, tmp, fd)  0 ||
+(tmp != IVSHMEM_PROTOCOL_VERSION) || fd != -1) {
+debug_log(client, cannot read from server\n);
+goto err_close;
+}
+debug_log(client, our_id=%ld\n, client-local.id);
+
+/* then, we expect our index + a fd == -1 */
 if (read_one_msg(client, client-local.id, fd)  0 ||
 client-local.id  0 || fd != -1) {
-debug_log(client, cannot read from server\n);
+debug_log(client, cannot read from server (2)\n);
 goto err_close;
 }
 debug_log(client, our_id=%ld\n, client-local.id);
@@ -196,7 +204,7 @@ ivshmem_client_connect(IvshmemClient *client)
  * is not used */
 if (read_one_msg(client, tmp, fd)  0 ||
 tmp != -1 || fd  0) {
-debug_log(client, cannot read from server (2)\n);
+debug_log(client, cannot read from server (3)\n);
 goto err_close;
 }
 debug_log(client, shm_fd=%d\n, fd);
diff --git a/contrib/ivshmem-client/ivshmem-client.h 
b/contrib/ivshmem-client/ivshmem-client.h
index 45f2b64..8d6ab35 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 f441da7..670c58c 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -99,6 +99,13 @@ send_initial_info(IvshmemServer *server, IvshmemServerPeer 
*peer)
 {
 int ret;
 
+/* send our protool version first */
+ret = send_one_msg(peer-sock_fd, IVSHMEM_PROTOCOL_VERSION, -1);
+if (ret  0) {
+debug_log(server, cannot send version: %s\n, strerror(errno));
+return -1;
+}
+
 /* send the peer id to the client */
 ret = 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 5ccc7af..e76e4fe 100644
--- a/contrib/ivshmem-server/ivshmem-server.h
+++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -30,6 +30,7 @@
 #include sys/select.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 f5f2b95..bae87de 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 an unix
 socket.
 
 For each client (QEMU processes) 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 make 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 make 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

[PATCH v4 03/14] contrib/ivshmem-*: comply with QEMU coding style

2014-09-02 Thread David Marchand
Fix coding style for structures.

Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/ivshmem-client.c |   47 ++-
 contrib/ivshmem-client/ivshmem-client.h |   76 +++
 contrib/ivshmem-client/main.c   |   21 -
 contrib/ivshmem-server/ivshmem-server.c |   38 
 contrib/ivshmem-server/ivshmem-server.h |   68 +--
 contrib/ivshmem-server/main.c   |   12 ++---
 6 files changed, 129 insertions(+), 133 deletions(-)

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index 2166b64..3f6ca98 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -31,7 +31,7 @@
 
 /* read message from the unix socket */
 static int
-read_one_msg(struct ivshmem_client *client, long *index, int *fd)
+read_one_msg(IvshmemClient *client, long *index, int *fd)
 {
 int ret;
 struct msghdr msg;
@@ -80,7 +80,7 @@ read_one_msg(struct ivshmem_client *client, long *index, int 
*fd)
 /* free a peer when the server advertise a disconnection or when the
  * client is freed */
 static void
-free_peer(struct ivshmem_client *client, struct ivshmem_client_peer *peer)
+free_peer(IvshmemClient *client, IvshmemClientPeer *peer)
 {
 unsigned vector;
 
@@ -94,9 +94,9 @@ free_peer(struct ivshmem_client *client, struct 
ivshmem_client_peer *peer)
 
 /* handle message coming from server (new peer, new vectors) */
 static int
-handle_server_msg(struct ivshmem_client *client)
+handle_server_msg(IvshmemClient *client)
 {
-struct ivshmem_client_peer *peer;
+IvshmemClientPeer *peer;
 long peer_id;
 int ret, fd;
 
@@ -146,7 +146,7 @@ handle_server_msg(struct ivshmem_client *client)
 
 /* init a new ivshmem client */
 int
-ivshmem_client_init(struct ivshmem_client *client, const char *unix_sock_path,
+ivshmem_client_init(IvshmemClient *client, const char *unix_sock_path,
 ivshmem_client_notif_cb_t notif_cb, void *notif_arg,
 int verbose)
 {
@@ -173,7 +173,7 @@ ivshmem_client_init(struct ivshmem_client *client, const 
char *unix_sock_path,
 
 /* create and connect to the unix socket */
 int
-ivshmem_client_connect(struct ivshmem_client *client)
+ivshmem_client_connect(IvshmemClient *client)
 {
 struct sockaddr_un sun;
 int fd;
@@ -223,9 +223,9 @@ ivshmem_client_connect(struct ivshmem_client *client)
 
 /* close connection to the server, and free all peer structures */
 void
-ivshmem_client_close(struct ivshmem_client *client)
+ivshmem_client_close(IvshmemClient *client)
 {
-struct ivshmem_client_peer *peer;
+IvshmemClientPeer *peer;
 unsigned i;
 
 debug_log(client, close client\n);
@@ -244,8 +244,7 @@ ivshmem_client_close(struct ivshmem_client *client)
 
 /* get the fd_set according to the unix socket and peer list */
 void
-ivshmem_client_get_fds(const struct ivshmem_client *client, fd_set *fds,
-   int *maxfd)
+ivshmem_client_get_fds(const IvshmemClient *client, fd_set *fds, int *maxfd)
 {
 int fd;
 unsigned vector;
@@ -266,9 +265,9 @@ ivshmem_client_get_fds(const struct ivshmem_client *client, 
fd_set *fds,
 
 /* handle events from eventfd: just print a message on notification */
 static int
-handle_event(struct ivshmem_client *client, const fd_set *cur, int maxfd)
+handle_event(IvshmemClient *client, const fd_set *cur, int maxfd)
 {
-struct ivshmem_client_peer *peer;
+IvshmemClientPeer *peer;
 uint64_t kick;
 unsigned i;
 int ret;
@@ -301,7 +300,7 @@ handle_event(struct ivshmem_client *client, const fd_set 
*cur, int maxfd)
 
 /* read and handle new messages on the given fd_set */
 int
-ivshmem_client_handle_fds(struct ivshmem_client *client, fd_set *fds, int 
maxfd)
+ivshmem_client_handle_fds(IvshmemClient *client, fd_set *fds, int maxfd)
 {
 if (client-sock_fd  maxfd  FD_ISSET(client-sock_fd, fds) 
 handle_server_msg(client)  0  errno != EINTR) {
@@ -317,8 +316,8 @@ ivshmem_client_handle_fds(struct ivshmem_client *client, 
fd_set *fds, int maxfd)
 
 /* send a notification on a vector of a peer */
 int
-ivshmem_client_notify(const struct ivshmem_client *client,
-  const struct ivshmem_client_peer *peer, unsigned vector)
+ivshmem_client_notify(const IvshmemClient *client,
+  const IvshmemClientPeer *peer, unsigned vector)
 {
 uint64_t kick;
 int fd;
@@ -342,8 +341,8 @@ ivshmem_client_notify(const struct ivshmem_client *client,
 
 /* send a notification to all vectors of a peer */
 int
-ivshmem_client_notify_all_vects(const struct ivshmem_client *client,
-const struct ivshmem_client_peer *peer)
+ivshmem_client_notify_all_vects(const IvshmemClient *client,
+const IvshmemClientPeer *peer)
 {
 unsigned vector;
 int ret = 0;
@@ -359,9 +358,9 @@ ivshmem_client_notify_all_vects

[PATCH v4 01/14] contrib: add ivshmem client and server

2014-09-02 Thread David Marchand
When using ivshmem devices, notifications between guests can be sent as
interrupts using a ivshmem-server (typical use described in documentation).
The client is provided as a debug tool.

Signed-off-by: Olivier Matz olivier.m...@6wind.com
Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/Makefile |   29 +++
 contrib/ivshmem-client/ivshmem-client.c |  418 ++
 contrib/ivshmem-client/ivshmem-client.h |  238 ++
 contrib/ivshmem-client/main.c   |  246 ++
 contrib/ivshmem-server/Makefile |   29 +++
 contrib/ivshmem-server/ivshmem-server.c |  420 +++
 contrib/ivshmem-server/ivshmem-server.h |  185 ++
 contrib/ivshmem-server/main.c   |  296 ++
 qemu-doc.texi   |   10 +-
 9 files changed, 1868 insertions(+), 3 deletions(-)
 create mode 100644 contrib/ivshmem-client/Makefile
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/Makefile
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

diff --git a/contrib/ivshmem-client/Makefile b/contrib/ivshmem-client/Makefile
new file mode 100644
index 000..eee97c6
--- /dev/null
+++ b/contrib/ivshmem-client/Makefile
@@ -0,0 +1,29 @@
+# Copyright 6WIND S.A., 2014
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# (at your option) any later version.  See the COPYING file in the
+# top-level directory.
+
+S ?= $(CURDIR)
+O ?= $(CURDIR)
+
+CFLAGS += -Wall -Wextra -Werror -g
+LDFLAGS +=
+LDLIBS += -lrt
+
+VPATH = $(S)
+PROG = ivshmem-client
+OBJS := $(O)/ivshmem-client.o
+OBJS += $(O)/main.o
+
+$(O)/%.o: %.c
+   $(CC) $(CFLAGS) -o $@ -c $
+
+$(O)/$(PROG): $(OBJS)
+   $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS)
+
+.PHONY: all
+all: $(O)/$(PROG)
+
+clean:
+   rm -f $(OBJS) $(O)/$(PROG)
diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
new file mode 100644
index 000..2166b64
--- /dev/null
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -0,0 +1,418 @@
+/*
+ * Copyright 6WIND S.A., 2014
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include stdio.h
+#include stdint.h
+#include stdlib.h
+#include errno.h
+#include string.h
+#include signal.h
+#include unistd.h
+#include inttypes.h
+#include sys/queue.h
+
+#include sys/types.h
+#include sys/socket.h
+#include sys/un.h
+
+#include ivshmem-client.h
+
+/* log a message on stdout if verbose=1 */
+#define debug_log(client, fmt, ...) do { \
+if ((client)-verbose) { \
+printf(fmt, ## __VA_ARGS__); \
+}\
+} while (0)
+
+/* read message from the unix socket */
+static int
+read_one_msg(struct ivshmem_client *client, long *index, int *fd)
+{
+int ret;
+struct msghdr msg;
+struct iovec iov[1];
+union {
+struct cmsghdr cmsg;
+char control[CMSG_SPACE(sizeof(int))];
+} msg_control;
+struct cmsghdr *cmsg;
+
+iov[0].iov_base = index;
+iov[0].iov_len = sizeof(*index);
+
+memset(msg, 0, sizeof(msg));
+msg.msg_iov = iov;
+msg.msg_iovlen = 1;
+msg.msg_control = msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+ret = recvmsg(client-sock_fd, msg, 0);
+if (ret  0) {
+debug_log(client, cannot read message: %s\n, strerror(errno));
+return -1;
+}
+if (ret == 0) {
+debug_log(client, lost connection to server\n);
+return -1;
+}
+
+*fd = -1;
+
+for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+
+if (cmsg-cmsg_len != CMSG_LEN(sizeof(int)) ||
+cmsg-cmsg_level != SOL_SOCKET ||
+cmsg-cmsg_type != SCM_RIGHTS) {
+continue;
+}
+
+memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd));
+}
+
+return 0;
+}
+
+/* free a peer when the server advertise a disconnection or when the
+ * client is freed */
+static void
+free_peer(struct ivshmem_client *client, struct ivshmem_client_peer *peer)
+{
+unsigned vector;
+
+TAILQ_REMOVE(client-peer_list, peer, next);
+for (vector = 0; vector  peer-vectors_count; vector++) {
+close(peer-vectors[vector]);
+}
+
+free(peer);
+}
+
+/* handle message coming from server (new peer, new vectors) */
+static int
+handle_server_msg(struct ivshmem_client *client)
+{
+struct ivshmem_client_peer *peer;
+long peer_id;
+int ret, fd;
+
+ret = read_one_msg(client, peer_id, fd);
+if (ret  0) {
+return -1;
+}
+
+/* can return

[PATCH v4 08/14] contrib/ivshmem-*: plug client and server in QEMU top Makefile

2014-09-02 Thread David Marchand
Signed-off-by: David Marchand david.march...@6wind.com
---
 Makefile|8 
 configure   |3 +++
 contrib/ivshmem-client/Makefile |   29 -
 contrib/ivshmem-server/Makefile |   29 -
 4 files changed, 11 insertions(+), 58 deletions(-)
 delete mode 100644 contrib/ivshmem-client/Makefile
 delete mode 100644 contrib/ivshmem-server/Makefile

diff --git a/Makefile b/Makefile
index b33aaac..0575898 100644
--- a/Makefile
+++ b/Makefile
@@ -283,6 +283,14 @@ $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
$(call LINK, $^)
 
+IVSHMEM_CLIENT_OBJS=$(addprefix $(SRC_PATH)/contrib/ivshmem-client/, 
ivshmem-client.o main.o)
+ivshmem-client$(EXESUF): $(IVSHMEM_CLIENT_OBJS)
+   $(call LINK, $^)
+
+IVSHMEM_SERVER_OBJS=$(addprefix $(SRC_PATH)/contrib/ivshmem-server/, 
ivshmem-server.o main.o)
+ivshmem-server$(EXESUF): $(IVSHMEM_SERVER_OBJS) libqemuutil.a libqemustub.a
+   $(call LINK, $^)
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
diff --git a/configure b/configure
index 961bf6f..a41a16c 100755
--- a/configure
+++ b/configure
@@ -4125,6 +4125,9 @@ if test $want_tools = yes ; then
   if [ $linux = yes -o $bsd = yes -o $solaris = yes ] ; then
 tools=qemu-nbd\$(EXESUF) $tools
   fi
+  if [ $kvm = yes ] ; then
+tools=ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools
+  fi
 fi
 if test $softmmu = yes ; then
   if test $virtfs != no ; then
diff --git a/contrib/ivshmem-client/Makefile b/contrib/ivshmem-client/Makefile
deleted file mode 100644
index eee97c6..000
--- a/contrib/ivshmem-client/Makefile
+++ /dev/null
@@ -1,29 +0,0 @@
-# Copyright 6WIND S.A., 2014
-#
-# This work is licensed under the terms of the GNU GPL, version 2 or
-# (at your option) any later version.  See the COPYING file in the
-# top-level directory.
-
-S ?= $(CURDIR)
-O ?= $(CURDIR)
-
-CFLAGS += -Wall -Wextra -Werror -g
-LDFLAGS +=
-LDLIBS += -lrt
-
-VPATH = $(S)
-PROG = ivshmem-client
-OBJS := $(O)/ivshmem-client.o
-OBJS += $(O)/main.o
-
-$(O)/%.o: %.c
-   $(CC) $(CFLAGS) -o $@ -c $
-
-$(O)/$(PROG): $(OBJS)
-   $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS)
-
-.PHONY: all
-all: $(O)/$(PROG)
-
-clean:
-   rm -f $(OBJS) $(O)/$(PROG)
diff --git a/contrib/ivshmem-server/Makefile b/contrib/ivshmem-server/Makefile
deleted file mode 100644
index 26b4a72..000
--- a/contrib/ivshmem-server/Makefile
+++ /dev/null
@@ -1,29 +0,0 @@
-# Copyright 6WIND S.A., 2014
-#
-# This work is licensed under the terms of the GNU GPL, version 2 or
-# (at your option) any later version.  See the COPYING file in the
-# top-level directory.
-
-S ?= $(CURDIR)
-O ?= $(CURDIR)
-
-CFLAGS += -Wall -Wextra -Werror -g
-LDFLAGS +=
-LDLIBS += -lrt
-
-VPATH = $(S)
-PROG = ivshmem-server
-OBJS := $(O)/ivshmem-server.o
-OBJS += $(O)/main.o
-
-$(O)/%.o: %.c
-   $(CC) $(CFLAGS) -o $@ -c $
-
-$(O)/$(PROG): $(OBJS)
-   $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS)
-
-.PHONY: all
-all: $(O)/$(PROG)
-
-clean:
-   rm -f $(OBJS) $(O)/$(PROG)
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 07/14] contrib/ivshmem-*: add missing const and static attrs

2014-09-02 Thread David Marchand
Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/main.c |6 +++---
 contrib/ivshmem-server/main.c |8 
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/ivshmem-client/main.c b/contrib/ivshmem-client/main.c
index f8a7b66..a8e1586 100644
--- a/contrib/ivshmem-client/main.c
+++ b/contrib/ivshmem-client/main.c
@@ -15,7 +15,7 @@
 
 typedef struct IvshmemClientArgs {
 bool verbose;
-char *unix_sock_path;
+const char *unix_sock_path;
 } IvshmemClientArgs;
 
 /* show usage and exit with given error code */
@@ -129,7 +129,7 @@ handle_stdin_command(IvshmemClient *client)
 
 /* listen on stdin (command line), on unix socket (notifications of new
  * and dead peers), and on eventfd (IRQ request) */
-int
+static int
 poll_events(IvshmemClient *client)
 {
 fd_set fds;
@@ -172,7 +172,7 @@ poll_events(IvshmemClient *client)
 }
 
 /* callback when we receive a notification (just display it) */
-void
+static void
 notification_cb(const IvshmemClient *client, const IvshmemClientPeer *peer,
 unsigned vect, void *arg)
 {
diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
index f00e6f9..1966e71 100644
--- a/contrib/ivshmem-server/main.c
+++ b/contrib/ivshmem-server/main.c
@@ -22,9 +22,9 @@
 typedef struct IvshmemServerArgs {
 bool verbose;
 bool foreground;
-char *pid_file;
-char *unix_socket_path;
-char *shm_path;
+const char *pid_file;
+const char *unix_socket_path;
+const char *shm_path;
 size_t shm_size;
 unsigned n_vectors;
 } IvshmemServerArgs;
@@ -138,7 +138,7 @@ parse_args(IvshmemServerArgs *args, int argc, char *argv[])
 
 /* wait for events on listening server unix socket and connected client
  * sockets */
-int
+static int
 poll_events(IvshmemServer *server)
 {
 fd_set fds;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 13/14] contrib/ivshmem-server: align server default parameter values

2014-09-02 Thread David Marchand
ivshmem server should use the same default values as hw/misc/ivshmem.
Update accordingly.

Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-server/main.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
index 1966e71..8792fc8 100644
--- a/contrib/ivshmem-server/main.c
+++ b/contrib/ivshmem-server/main.c
@@ -15,8 +15,8 @@
 #define DEFAULT_PID_FILE   /var/run/ivshmem-server.pid
 #define DEFAULT_UNIX_SOCK_PATH /tmp/ivshmem_socket
 #define DEFAULT_SHM_PATH   ivshmem
-#define DEFAULT_SHM_SIZE   (1024*1024)
-#define DEFAULT_N_VECTORS  16
+#define DEFAULT_SHM_SIZE   (4*1024*1024)
+#define DEFAULT_N_VECTORS  1
 
 /* arguments given by the user */
 typedef struct IvshmemServerArgs {
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 05/14] contrib/ivshmem-*: switch to QEMU headers

2014-09-02 Thread David Marchand
Reuse parsers from QEMU, C99 boolean.

Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/ivshmem-client.c |   12 +
 contrib/ivshmem-client/ivshmem-client.h |4 +-
 contrib/ivshmem-client/main.c   |   12 +
 contrib/ivshmem-server/ivshmem-server.c |   14 +-
 contrib/ivshmem-server/ivshmem-server.h |4 +-
 contrib/ivshmem-server/main.c   |   73 +--
 6 files changed, 20 insertions(+), 99 deletions(-)

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index ce3a5d2..2ba40a7 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -6,19 +6,11 @@
  * top-level directory.
  */
 
-#include stdio.h
-#include stdint.h
-#include stdlib.h
-#include errno.h
-#include string.h
-#include signal.h
-#include unistd.h
-#include inttypes.h
-
 #include sys/types.h
 #include sys/socket.h
 #include sys/un.h
 
+#include qemu-common.h
 #include qemu/queue.h
 
 #include ivshmem-client.h
@@ -149,7 +141,7 @@ handle_server_msg(IvshmemClient *client)
 int
 ivshmem_client_init(IvshmemClient *client, const char *unix_sock_path,
 ivshmem_client_notif_cb_t notif_cb, void *notif_arg,
-int verbose)
+bool verbose)
 {
 unsigned i;
 
diff --git a/contrib/ivshmem-client/ivshmem-client.h 
b/contrib/ivshmem-client/ivshmem-client.h
index e3b284d..45f2b64 100644
--- a/contrib/ivshmem-client/ivshmem-client.h
+++ b/contrib/ivshmem-client/ivshmem-client.h
@@ -78,7 +78,7 @@ struct IvshmemClient {
 ivshmem_client_notif_cb_t notif_cb; /** notification callback */
 void *notif_arg;/** notification argument */
 
-int verbose;/** true to enable debug */
+bool verbose;   /** true to enable debug */
 };
 
 /**
@@ -101,7 +101,7 @@ struct IvshmemClient {
  */
 int ivshmem_client_init(IvshmemClient *client, const char *unix_sock_path,
 ivshmem_client_notif_cb_t notif_cb, void *notif_arg,
-int verbose);
+bool verbose);
 
 /**
  * Connect to the server
diff --git a/contrib/ivshmem-client/main.c b/contrib/ivshmem-client/main.c
index 778d0f2..f8a7b66 100644
--- a/contrib/ivshmem-client/main.c
+++ b/contrib/ivshmem-client/main.c
@@ -6,15 +6,7 @@
  * top-level directory.
  */
 
-#include stdio.h
-#include stdint.h
-#include stdlib.h
-#include errno.h
-#include string.h
-#include signal.h
-#include unistd.h
-#include inttypes.h
-#include getopt.h
+#include qemu-common.h
 
 #include ivshmem-client.h
 
@@ -22,7 +14,7 @@
 #define DEFAULT_UNIX_SOCK_PATH /tmp/ivshmem_socket
 
 typedef struct IvshmemClientArgs {
-int verbose;
+bool verbose;
 char *unix_sock_path;
 } IvshmemClientArgs;
 
diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index e58864d..0afa6e8 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -6,23 +6,13 @@
  * top-level directory.
  */
 
-#include stdio.h
-#include stdint.h
-#include stdlib.h
-#include errno.h
-#include string.h
-#include signal.h
-#include unistd.h
-#include inttypes.h
-#include fcntl.h
-
 #include sys/mman.h
-#include sys/stat.h
 #include sys/types.h
 #include sys/socket.h
 #include sys/un.h
 #include sys/eventfd.h
 
+#include qemu-common.h
 #include qemu/queue.h
 
 #include ivshmem-server.h
@@ -246,7 +236,7 @@ ivshmem_ftruncate(int fd, unsigned shmsize)
 int
 ivshmem_server_init(IvshmemServer *server, const char *unix_sock_path,
 const char *shm_path, size_t shm_size, unsigned n_vectors,
-int verbose)
+bool verbose)
 {
 memset(server, 0, sizeof(*server));
 
diff --git a/contrib/ivshmem-server/ivshmem-server.h 
b/contrib/ivshmem-server/ivshmem-server.h
index acd17a8..5ccc7af 100644
--- a/contrib/ivshmem-server/ivshmem-server.h
+++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -69,7 +69,7 @@ typedef struct IvshmemServer {
 int shm_fd;  /** shm file descriptor */
 unsigned n_vectors;  /** number of vectors */
 long cur_id; /** id to be given to next client */
-int verbose; /** true in verbose mode */
+bool verbose;/** true in verbose mode */
 IvshmemServerPeerList peer_list; /** list of peers */
 } IvshmemServer;
 
@@ -97,7 +97,7 @@ typedef struct IvshmemServer {
 int
 ivshmem_server_init(IvshmemServer *server, const char *unix_sock_path,
 const char *shm_path, size_t shm_size, unsigned n_vectors,
-int verbose);
+bool verbose);
 
 /**
  * Open the shm, then create and bind to the unix socket
diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
index a4504c3..f00e6f9 100644

Re: [Qemu-devel] [PATCH v3 2/2] docs: update ivshmem device spec

2014-09-01 Thread David Marchand

On 08/28/2014 11:49 AM, Stefan Hajnoczi wrote:

On Tue, Aug 26, 2014 at 01:04:30PM +0200, Paolo Bonzini wrote:

Il 26/08/2014 08:47, David Marchand ha scritto:


Using a version message supposes we want to keep ivshmem-server and QEMU
separated (for example, in two distribution packages) while we can avoid
this, so why would we do so ?

If we want the ivshmem-server to come with QEMU, then both are supposed
to be aligned on your system.


What about upgrading QEMU and ivshmem-server while you have existing
guests?  You cannot restart ivshmem-server, and the new QEMU would have
to talk to the old ivshmem-server.


Version negotiation also helps avoid confusion if someone combines
ivshmem-server and QEMU from different origins (e.g. built from source
and distro packaged).

It's a safeguard to prevent hard-to-diagnose failures when the system is
misconfigured.



Hum, so you want the code to be defensive against mis-use, why not.

I wanted to keep modifications on ivshmem as little as possible in a 
first phase (all the more so as there are potential ivshmem users out 
there that I think will be impacted by a protocol change).


Sending the version as the first vm_id with an associated fd to -1 
before sending the real client id should work with existing QEMU client 
code (hw/misc/ivshmem.c).


Do you have a better idea ?
Is there a best practice in QEMU for version negotiation that could 
work with ivshmem protocol ?


I have a v4 ready with this (and all the pending comments), I will send 
it later unless a better idea is exposed.



Thanks.

--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [PATCH v3 2/2] docs: update ivshmem device spec

2014-08-26 Thread David Marchand

Hello Stefan,

On 08/08/2014 05:02 PM, Stefan Hajnoczi wrote:

On Fri, Aug 08, 2014 at 10:55:18AM +0200, David Marchand wrote:

+For each client (QEMU processes) that connects to the server:
+- 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,
+- the server creates a new set of host eventfds associated to the new client 
and
+  sends this set to all already connected clients,
+- finally, the server sends all the eventfds sets for all clients to the new
+  client.


The protocol is not extensible and no version number is exchanged.  For
the most part this should be okay because clients must run on the same
machine as the server.  It is assumed clients and server are compatible
with each other.

I wonder if we'll get into trouble later if the protocol needs to be
extended or some operation needs to happen, like upgrading QEMU or the
ivshmem-server.  At the very least someone building from source but
using system QEMU or ivshmem-server could get confusing failures if the
protocol doesn't match.

How about sending a version message as the first thing during a
connection?


I am not too sure about this.

This would break current base version.

Using a version message supposes we want to keep ivshmem-server and QEMU 
separated (for example, in two distribution packages) while we can avoid 
this, so why would we do so ?


If we want the ivshmem-server to come with QEMU, then both are supposed 
to be aligned on your system.
If you want to test local modifications, then it means you know what you 
are doing and you will call the right ivshmem-server binary with the 
right QEMU binary.



Thanks.


--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [PATCH v3 1/2] contrib: add ivshmem client and server

2014-08-18 Thread David Marchand


On 08/08/2014 04:51 PM, Stefan Hajnoczi wrote:

On Fri, Aug 08, 2014 at 10:55:17AM +0200, David Marchand wrote:

Looks good, a few minor comments:


diff --git a/contrib/ivshmem-client/Makefile b/contrib/ivshmem-client/Makefile
new file mode 100644
index 000..eee97c6
--- /dev/null
+++ b/contrib/ivshmem-client/Makefile
@@ -0,0 +1,29 @@


CCed Peter Maydell for a second opinion, I'd suggest hooking up to
QEMU's top-level ./Makefile.  QEMU does not do recursive make.

The advantages of hooking up QEMU's Makefile are:

1. So that ivshmem client/server code is built by default (on supported
host platforms) and bitrot is avoided.

2. So that you don't have to duplicate rules.mak or any other build
infrastructure.



Ok, done.
But in this case, should we really keep the files in contrib/ ?
I used this directory but I am not too sure about this.

Maybe hw/misc/ivshmem/ would be better ?


The rest of your comments have been integrated in my tree.

Is it preferred to send fixes aside the original patches ? or should I 
send a squashed version ?

(I suppose the former is better as it is easier to read).


Thanks Stefan.


--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [PATCH v3 1/2] contrib: add ivshmem client and server

2014-08-18 Thread David Marchand

On 08/10/2014 05:57 AM, Gonglei wrote:

+/* can return a peer or the local client */
+peer = ivshmem_client_search_peer(client, peer_id);
+
+/* delete peer */
+if (fd == -1) {
+

Maybe the above check should be moved before getting the peer.
And the next check peer is extra.


We always need to know the peer, either for a deletion, creation or update.





+if (peer == NULL || peer == client-local) {
+debug_log(client, receive delete for invalid peer %ld,


Missing '\n' ?


Ok.




peer_id);
+return -1;
+}
+
+debug_log(client, delete peer id = %ld\n, peer_id);
+free_peer(client, peer);
+return 0;
+}
+
+/* new peer */
+if (peer == NULL) {
+peer = malloc(sizeof(*peer));


g_malloc0 ?.


Ok, replaced malloc/free with g_malloc/g_free in client and server.



+client-notif_cb = notif_cb;
+client-notif_arg = notif_arg;
+client-verbose = verbose;


Missing client-sock_fd = -1; ?


Ok.



+
+/* first, we expect our index + a fd == -1 */
+if (read_one_msg(client, client-local.id, fd)  0 ||
+client-local.id  0 || fd != -1) {

Why not fd  0 ?


Because the server will send us an id and a fd == -1 see 
ivshmem-server.c send_initial_info().





+debug_log(client, cannot read from server\n);
+close(client-sock_fd);
+client-sock_fd = -1;
+return -1;
+}
+debug_log(client, our_id=%ld\n, client-local.id);
+
+/* now, we expect shared mem fd + a -1 index, note that shm fd
+ * is not used */
+if (read_one_msg(client, tmp, fd)  0 ||
+tmp != -1 || fd  0) {
+debug_log(client, cannot read from server (2)\n);
+close(client-sock_fd);
+client-sock_fd = -1;
+return -1;

I think the error logic handle can move the end of this function, reducing
duplicated code. Something like this:

goto err;
   }
err:
debug_log(client, cannot read from server (2)\n);
 close(client-sock_fd);
 client-sock_fd = -1;
 return -1;


Ok, I also updated the server.


+int fd;
+
+if (vector  peer-vectors_count) {


Maybe if (vector = peer-vectors_count) , otherwise the
peer-vectors[] array bounds.


Oh yes, good catch.
It should not happen, at the moment, but it is wrong, indeed.


+/* send a notification to all vectors of a peer */
+int
+ivshmem_client_notify_all_vects(const struct ivshmem_client *client,
+const struct ivshmem_client_peer
*peer)
+{
+unsigned vector;
+int ret = 0;
+
+for (vector = 0; vector  peer-vectors_count; vector++) {
+if (ivshmem_client_notify(client, peer, vector)  0) {
+ret = -1;

The ret's value will be covered when multi clients failed. Do we need
store the failed status for server?.


It indicates that we could not notify *all* clients.



Thanks Gonglei.


--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [PATCH v2 1/2] contrib: add ivshmem client and server

2014-08-08 Thread David Marchand

Hello Markus,

On 07/21/2014 07:35 PM, Markus Armbruster wrote:


Do you have a compelling reason why you can't license under GPLv2+?  If
yes, please explain it to us.  If no, please use

  * This work is licensed under the terms of the GNU GPL, version 2 or
  * later.  See the COPYING file in the top-level directory.

[...]


Back from holidays, sorry for the late response.

I will send a v3 for these issues.


--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/2] ivshmem: update documentation, add client/server tools

2014-08-08 Thread David Marchand
Here is a patchset containing an update on ivshmem specs documentation and
importing ivshmem server and client tools.
These tools have been written from scratch and are not related to what is
available in nahanni repository.
I put them in contrib/ directory as the qemu-doc.texi was already telling the
server was supposed to be there.

Changes since v2:
- fixed license issues in ivshmem client/server (I took hw/virtio/virtio-rng.c
  file as a reference).

Changes since v1:
- moved client/server import patch before doc update,
- tried to re-organise the ivshmem_device_spec.txt file based on Claudio
  comments (still not sure if the result is that great, comments welcome),
- incorporated comments from Claudio, Eric and Cam,
- added more details on the server - client messages exchange (but sorry, no
  ASCII art here).

By the way, there are still some functionnalities that need description (use of
ioeventfd, the lack of irqfd support) and some parts of the ivshmem code clearly
need cleanup. I will try to address this in future patches when these first
patches are ok.


-- 
David Marchand

David Marchand (2):
  contrib: add ivshmem client and server
  docs: update ivshmem device spec

 contrib/ivshmem-client/Makefile |   29 +++
 contrib/ivshmem-client/ivshmem-client.c |  418 ++
 contrib/ivshmem-client/ivshmem-client.h |  238 ++
 contrib/ivshmem-client/main.c   |  246 ++
 contrib/ivshmem-server/Makefile |   29 +++
 contrib/ivshmem-server/ivshmem-server.c |  420 +++
 contrib/ivshmem-server/ivshmem-server.h |  185 ++
 contrib/ivshmem-server/main.c   |  296 ++
 docs/specs/ivshmem_device_spec.txt  |  124 ++---
 qemu-doc.texi   |   10 +-
 10 files changed, 1961 insertions(+), 34 deletions(-)
 create mode 100644 contrib/ivshmem-client/Makefile
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/Makefile
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/2] contrib: add ivshmem client and server

2014-08-08 Thread David Marchand
When using ivshmem devices, notifications between guests can be sent as
interrupts using a ivshmem-server (typical use described in documentation).
The client is provided as a debug tool.

Signed-off-by: Olivier Matz olivier.m...@6wind.com
Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/Makefile |   29 +++
 contrib/ivshmem-client/ivshmem-client.c |  418 ++
 contrib/ivshmem-client/ivshmem-client.h |  238 ++
 contrib/ivshmem-client/main.c   |  246 ++
 contrib/ivshmem-server/Makefile |   29 +++
 contrib/ivshmem-server/ivshmem-server.c |  420 +++
 contrib/ivshmem-server/ivshmem-server.h |  185 ++
 contrib/ivshmem-server/main.c   |  296 ++
 qemu-doc.texi   |   10 +-
 9 files changed, 1868 insertions(+), 3 deletions(-)
 create mode 100644 contrib/ivshmem-client/Makefile
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/Makefile
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

diff --git a/contrib/ivshmem-client/Makefile b/contrib/ivshmem-client/Makefile
new file mode 100644
index 000..eee97c6
--- /dev/null
+++ b/contrib/ivshmem-client/Makefile
@@ -0,0 +1,29 @@
+# Copyright 6WIND S.A., 2014
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# (at your option) any later version.  See the COPYING file in the
+# top-level directory.
+
+S ?= $(CURDIR)
+O ?= $(CURDIR)
+
+CFLAGS += -Wall -Wextra -Werror -g
+LDFLAGS +=
+LDLIBS += -lrt
+
+VPATH = $(S)
+PROG = ivshmem-client
+OBJS := $(O)/ivshmem-client.o
+OBJS += $(O)/main.o
+
+$(O)/%.o: %.c
+   $(CC) $(CFLAGS) -o $@ -c $
+
+$(O)/$(PROG): $(OBJS)
+   $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS)
+
+.PHONY: all
+all: $(O)/$(PROG)
+
+clean:
+   rm -f $(OBJS) $(O)/$(PROG)
diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
new file mode 100644
index 000..2166b64
--- /dev/null
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -0,0 +1,418 @@
+/*
+ * Copyright 6WIND S.A., 2014
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include stdio.h
+#include stdint.h
+#include stdlib.h
+#include errno.h
+#include string.h
+#include signal.h
+#include unistd.h
+#include inttypes.h
+#include sys/queue.h
+
+#include sys/types.h
+#include sys/socket.h
+#include sys/un.h
+
+#include ivshmem-client.h
+
+/* log a message on stdout if verbose=1 */
+#define debug_log(client, fmt, ...) do { \
+if ((client)-verbose) { \
+printf(fmt, ## __VA_ARGS__); \
+}\
+} while (0)
+
+/* read message from the unix socket */
+static int
+read_one_msg(struct ivshmem_client *client, long *index, int *fd)
+{
+int ret;
+struct msghdr msg;
+struct iovec iov[1];
+union {
+struct cmsghdr cmsg;
+char control[CMSG_SPACE(sizeof(int))];
+} msg_control;
+struct cmsghdr *cmsg;
+
+iov[0].iov_base = index;
+iov[0].iov_len = sizeof(*index);
+
+memset(msg, 0, sizeof(msg));
+msg.msg_iov = iov;
+msg.msg_iovlen = 1;
+msg.msg_control = msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+ret = recvmsg(client-sock_fd, msg, 0);
+if (ret  0) {
+debug_log(client, cannot read message: %s\n, strerror(errno));
+return -1;
+}
+if (ret == 0) {
+debug_log(client, lost connection to server\n);
+return -1;
+}
+
+*fd = -1;
+
+for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+
+if (cmsg-cmsg_len != CMSG_LEN(sizeof(int)) ||
+cmsg-cmsg_level != SOL_SOCKET ||
+cmsg-cmsg_type != SCM_RIGHTS) {
+continue;
+}
+
+memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd));
+}
+
+return 0;
+}
+
+/* free a peer when the server advertise a disconnection or when the
+ * client is freed */
+static void
+free_peer(struct ivshmem_client *client, struct ivshmem_client_peer *peer)
+{
+unsigned vector;
+
+TAILQ_REMOVE(client-peer_list, peer, next);
+for (vector = 0; vector  peer-vectors_count; vector++) {
+close(peer-vectors[vector]);
+}
+
+free(peer);
+}
+
+/* handle message coming from server (new peer, new vectors) */
+static int
+handle_server_msg(struct ivshmem_client *client)
+{
+struct ivshmem_client_peer *peer;
+long peer_id;
+int ret, fd;
+
+ret = read_one_msg(client, peer_id, fd);
+if (ret  0) {
+return -1;
+}
+
+/* can return

[PATCH v3 2/2] docs: update ivshmem device spec

2014-08-08 Thread David Marchand
Add some notes on the parts needed to use ivshmem devices: more specifically,
explain the purpose of an ivshmem server and the basic concept to use the
ivshmem devices in guests.
Move some parts of the documentation and re-organise it.

Signed-off-by: David Marchand david.march...@6wind.com
---
 docs/specs/ivshmem_device_spec.txt |  124 +++-
 1 file changed, 93 insertions(+), 31 deletions(-)

diff --git a/docs/specs/ivshmem_device_spec.txt 
b/docs/specs/ivshmem_device_spec.txt
index 667a862..f5f2b95 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -2,30 +2,103 @@
 Device Specification for Inter-VM shared memory device
 --
 
-The Inter-VM shared memory device is designed to share a region of memory to
-userspace in multiple virtual guests.  The memory region does not belong to any
-guest, but is a POSIX memory object on the host.  Optionally, the device may
-support sending interrupts to other guests sharing the same memory region.
+The Inter-VM shared memory device is designed to share a memory region (created
+on the host via the POSIX shared memory API) between multiple QEMU processes
+running different guests. In order for all guests to be able to pick up the
+shared memory area, it is modeled by QEMU as a PCI device exposing said memory
+to the guest as a PCI BAR.
+The memory region does not belong to any guest, but is a POSIX memory object on
+the host. The host can access this shared memory if needed.
+
+The device also provides an optional communication mechanism between guests
+sharing the same memory object. More details about that in the section 'Guest 
to
+guest communication' section.
 
 
 The Inter-VM PCI device
 ---
 
-*BARs*
+From the VM point of view, the ivshmem PCI device supports three BARs.
+
+- BAR0 is a 1 Kbyte MMIO region to support registers and interrupts when MSI is
+  not used.
+- BAR1 is used for MSI-X when it is enabled in the device.
+- BAR2 is used to access the shared memory object.
+
+It is your choice how to use the device but you must choose between two
+behaviors :
+
+- basically, if you only need the shared memory part, you will map BAR2.
+  This way, you have access to the shared memory in guest and can use it as you
+  see fit (memnic, for example, uses it in userland
+  http://dpdk.org/browse/memnic).
+
+- BAR0 and BAR1 are used to implement an optional communication mechanism
+  through interrupts in the guests. If you need an event mechanism between the
+  guests accessing the shared memory, you will most likely want to write a
+  kernel driver that will handle interrupts. See details in the section 'Guest
+  to guest communication' section.
+
+The behavior is chosen when starting your QEMU processes:
+- no communication mechanism needed, the first QEMU to start creates the shared
+  memory on the host, subsequent QEMU processes will use it.
+
+- communication mechanism needed, an ivshmem server must be started before any
+  QEMU processes, then each QEMU process connects to the server unix socket.
+
+For more details on the QEMU ivshmem parameters, see qemu-doc documentation.
+
+
+Guest to guest communication
+
+
+This section details the communication mechanism between the guests accessing
+the ivhsmem shared memory.
 
-The device supports three BARs.  BAR0 is a 1 Kbyte MMIO region to support
-registers.  BAR1 is used for MSI-X when it is enabled in the device.  BAR2 is
-used to map the shared memory object from the host.  The size of BAR2 is
-specified when the guest is started and must be a power of 2 in size.
+*ivshmem server*
 
-*Registers*
+This server code is available in qemu.git/contrib/ivshmem-server.
 
-The device currently supports 4 registers of 32-bits each.  Registers
-are used for synchronization between guests sharing the same memory object when
-interrupts are supported (this requires using the shared memory server).
+The server must be started on the host before any guest.
+It creates a shared memory object then waits for clients to connect on an unix
+socket.
 
-The server assigns each VM an ID number and sends this ID number to the QEMU
-process when the guest starts.
+For each client (QEMU processes) that connects to the server:
+- 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,
+- the server creates a new set of host eventfds associated to the new client 
and
+  sends this set to all already connected clients,
+- finally, the server sends all the eventfds sets for all clients to the new
+  client.
+
+The server signals all clients when one of them disconnects.
+
+The client IDs are limited to 16 bits because of the current implementation 
(see
+Doorbell register in 'PCI device registers' subsection). Hence on 65536 clients
+are supported.
+
+All the file

Re: [PATCH v3 2/2] docs: update ivshmem device spec

2014-08-08 Thread David Marchand

Hello Claudio,

On 08/08/2014 11:04 AM, Claudio Fontana wrote:

On 08.08.2014 10:55, David Marchand wrote:

Add some notes on the parts needed to use ivshmem devices: more specifically,
explain the purpose of an ivshmem server and the basic concept to use the
ivshmem devices in guests.
Move some parts of the documentation and re-organise it.

Signed-off-by: David Marchand david.march...@6wind.com


You did not include my Reviewed-by: tag, did you change this from v2?



No, I did not change anything in the documentation patch, I only touched 
the client/server patch.


I forgot to add your Reviewed-by tag ... (added to my tree for now, will 
send a v4 when I have some feedback on the client/server code).



Thanks Claudio.


--
David Marchand

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [PATCH v3 0/2] ivshmem: update documentation, add client/server tools

2014-08-08 Thread David Marchand

Hello Gonglei,

On 08/08/2014 11:30 AM, Gonglei (Arei) wrote:

If you can describe the steps of using example about
your ivshmem-client and ivshmem-server will be great IMHO.


I already have included a note in the qemu-doc.texi file on how to start 
the ivshmem-server.

The (debug) client is started by only specifying -S /path/to/ivshmem_socket.

We made comments into the source code, so I am not sure what could be 
added. What do you miss ?



--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] contrib: add ivshmem client and server

2014-07-21 Thread David Marchand

Hello Eric,

On 07/21/2014 04:21 PM, Eric Blake wrote:

On 07/20/2014 03:38 AM, David Marchand wrote:

+# Copyright 2014 6WIND S.A.
+# All rights reserved


This file has no other license, and is therefore incompatible with
GPLv2.  You'll need to resubmit under an appropriately open license.


missed the makefiles ...




+++ b/contrib/ivshmem-client/ivshmem-client.h
@@ -0,0 +1,238 @@
+/*
+ * Copyright(c) 2014 6WIND S.A.
+ * All rights reserved.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.


I'm not a lawyer, but to me, this license is self-contradictory.  You
can't have All rights reserved and still be GPL, because the point of
the GPL is that you are NOT reserving all rights, but explicitly
granting your user various rights (on condition that they likewise grant
those rights to others).  But you're not the only file in the qemu code
base with this questionable mix.



Hum, ok, will update.


--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2] ivshmem: update documentation, add client/server tools

2014-07-20 Thread David Marchand
Here is a patchset containing an update on ivshmem specs documentation and
importing ivshmem server and client tools.
These tools have been written from scratch and are not related to what is
available in nahanni repository.
I put them in contrib/ directory as the qemu-doc.texi was already telling the
server was supposed to be there.

Changes since v1:
- moved client/server import patch before doc update,
- tried to re-organise the ivshmem_device_spec.txt file based on Claudio
  comments (still not sure if the result is that great, comments welcome),
- incorporated comments from Claudio, Eric and Cam,
- added more details on the server - client messages exchange (but sorry, no
  ASCII art here).

By the way, there are still some functionnalities that need description (use of
ioeventfd, the lack of irqfd support) and some parts of the ivshmem code clearly
need cleanup. I will try to address this in future patches when these first
patches are ok.


-- 
David Marchand

David Marchand (2):
  contrib: add ivshmem client and server
  docs: update ivshmem device spec

 contrib/ivshmem-client/Makefile |   26 ++
 contrib/ivshmem-client/ivshmem-client.c |  418 ++
 contrib/ivshmem-client/ivshmem-client.h |  238 ++
 contrib/ivshmem-client/main.c   |  246 ++
 contrib/ivshmem-server/Makefile |   26 ++
 contrib/ivshmem-server/ivshmem-server.c |  420 +++
 contrib/ivshmem-server/ivshmem-server.h |  185 ++
 contrib/ivshmem-server/main.c   |  296 ++
 docs/specs/ivshmem_device_spec.txt  |  124 ++---
 qemu-doc.texi   |   10 +-
 10 files changed, 1955 insertions(+), 34 deletions(-)
 create mode 100644 contrib/ivshmem-client/Makefile
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/Makefile
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] contrib: add ivshmem client and server

2014-07-20 Thread David Marchand
When using ivshmem devices, notifications between guests can be sent as
interrupts using a ivshmem-server (typical use described in documentation).
The client is provided as a debug tool.

Signed-off-by: Olivier Matz olivier.m...@6wind.com
Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/Makefile |   26 ++
 contrib/ivshmem-client/ivshmem-client.c |  418 ++
 contrib/ivshmem-client/ivshmem-client.h |  238 ++
 contrib/ivshmem-client/main.c   |  246 ++
 contrib/ivshmem-server/Makefile |   26 ++
 contrib/ivshmem-server/ivshmem-server.c |  420 +++
 contrib/ivshmem-server/ivshmem-server.h |  185 ++
 contrib/ivshmem-server/main.c   |  296 ++
 qemu-doc.texi   |   10 +-
 9 files changed, 1862 insertions(+), 3 deletions(-)
 create mode 100644 contrib/ivshmem-client/Makefile
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/Makefile
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

diff --git a/contrib/ivshmem-client/Makefile b/contrib/ivshmem-client/Makefile
new file mode 100644
index 000..9e32409
--- /dev/null
+++ b/contrib/ivshmem-client/Makefile
@@ -0,0 +1,26 @@
+# Copyright 2014 6WIND S.A.
+# All rights reserved
+
+S ?= $(CURDIR)
+O ?= $(CURDIR)
+
+CFLAGS += -Wall -Wextra -Werror -g
+LDFLAGS +=
+LDLIBS += -lrt
+
+VPATH = $(S)
+PROG = ivshmem-client
+OBJS := $(O)/ivshmem-client.o
+OBJS += $(O)/main.o
+
+$(O)/%.o: %.c
+   $(CC) $(CFLAGS) -o $@ -c $
+
+$(O)/$(PROG): $(OBJS)
+   $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS)
+
+.PHONY: all
+all: $(O)/$(PROG)
+
+clean:
+   rm -f $(OBJS) $(O)/$(PROG)
diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
new file mode 100644
index 000..32ef3ef
--- /dev/null
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -0,0 +1,418 @@
+/*
+ * Copyright(c) 2014 6WIND S.A.
+ * All rights reserved.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include stdio.h
+#include stdint.h
+#include stdlib.h
+#include errno.h
+#include string.h
+#include signal.h
+#include unistd.h
+#include inttypes.h
+#include sys/queue.h
+
+#include sys/types.h
+#include sys/socket.h
+#include sys/un.h
+
+#include ivshmem-client.h
+
+/* log a message on stdout if verbose=1 */
+#define debug_log(client, fmt, ...) do { \
+if ((client)-verbose) { \
+printf(fmt, ## __VA_ARGS__); \
+}\
+} while (0)
+
+/* read message from the unix socket */
+static int
+read_one_msg(struct ivshmem_client *client, long *index, int *fd)
+{
+int ret;
+struct msghdr msg;
+struct iovec iov[1];
+union {
+struct cmsghdr cmsg;
+char control[CMSG_SPACE(sizeof(int))];
+} msg_control;
+struct cmsghdr *cmsg;
+
+iov[0].iov_base = index;
+iov[0].iov_len = sizeof(*index);
+
+memset(msg, 0, sizeof(msg));
+msg.msg_iov = iov;
+msg.msg_iovlen = 1;
+msg.msg_control = msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+ret = recvmsg(client-sock_fd, msg, 0);
+if (ret  0) {
+debug_log(client, cannot read message: %s\n, strerror(errno));
+return -1;
+}
+if (ret == 0) {
+debug_log(client, lost connection to server\n);
+return -1;
+}
+
+*fd = -1;
+
+for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+
+if (cmsg-cmsg_len != CMSG_LEN(sizeof(int)) ||
+cmsg-cmsg_level != SOL_SOCKET ||
+cmsg-cmsg_type != SCM_RIGHTS) {
+continue;
+}
+
+memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd));
+}
+
+return 0;
+}
+
+/* free a peer when the server advertise a disconnection or when the
+ * client is freed */
+static void
+free_peer(struct ivshmem_client *client, struct ivshmem_client_peer *peer)
+{
+unsigned vector;
+
+TAILQ_REMOVE(client-peer_list, peer, next);
+for (vector = 0; vector  peer-vectors_count; vector++) {
+close(peer-vectors[vector]);
+}
+
+free(peer);
+}
+
+/* handle message coming from server (new peer, new vectors) */
+static int
+handle_server_msg(struct ivshmem_client *client)
+{
+struct ivshmem_client_peer *peer;
+long peer_id;
+int ret, fd;
+
+ret = read_one_msg(client, peer_id, fd);
+if (ret  0) {
+return -1;
+}
+
+/* can return a peer or the local client */
+peer = ivshmem_client_search_peer(client, peer_id);
+
+/* delete peer */
+if (fd == -1) {
+
+if (peer == NULL

[PATCH v2 2/2] docs: update ivshmem device spec

2014-07-20 Thread David Marchand
Add some notes on the parts needed to use ivshmem devices: more specifically,
explain the purpose of an ivshmem server and the basic concept to use the
ivshmem devices in guests.
Move some parts of the documentation and re-organise it.

Signed-off-by: David Marchand david.march...@6wind.com
---
 docs/specs/ivshmem_device_spec.txt |  124 +++-
 1 file changed, 93 insertions(+), 31 deletions(-)

diff --git a/docs/specs/ivshmem_device_spec.txt 
b/docs/specs/ivshmem_device_spec.txt
index 667a862..f5f2b95 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -2,30 +2,103 @@
 Device Specification for Inter-VM shared memory device
 --
 
-The Inter-VM shared memory device is designed to share a region of memory to
-userspace in multiple virtual guests.  The memory region does not belong to any
-guest, but is a POSIX memory object on the host.  Optionally, the device may
-support sending interrupts to other guests sharing the same memory region.
+The Inter-VM shared memory device is designed to share a memory region (created
+on the host via the POSIX shared memory API) between multiple QEMU processes
+running different guests. In order for all guests to be able to pick up the
+shared memory area, it is modeled by QEMU as a PCI device exposing said memory
+to the guest as a PCI BAR.
+The memory region does not belong to any guest, but is a POSIX memory object on
+the host. The host can access this shared memory if needed.
+
+The device also provides an optional communication mechanism between guests
+sharing the same memory object. More details about that in the section 'Guest 
to
+guest communication' section.
 
 
 The Inter-VM PCI device
 ---
 
-*BARs*
+From the VM point of view, the ivshmem PCI device supports three BARs.
+
+- BAR0 is a 1 Kbyte MMIO region to support registers and interrupts when MSI is
+  not used.
+- BAR1 is used for MSI-X when it is enabled in the device.
+- BAR2 is used to access the shared memory object.
+
+It is your choice how to use the device but you must choose between two
+behaviors :
+
+- basically, if you only need the shared memory part, you will map BAR2.
+  This way, you have access to the shared memory in guest and can use it as you
+  see fit (memnic, for example, uses it in userland
+  http://dpdk.org/browse/memnic).
+
+- BAR0 and BAR1 are used to implement an optional communication mechanism
+  through interrupts in the guests. If you need an event mechanism between the
+  guests accessing the shared memory, you will most likely want to write a
+  kernel driver that will handle interrupts. See details in the section 'Guest
+  to guest communication' section.
+
+The behavior is chosen when starting your QEMU processes:
+- no communication mechanism needed, the first QEMU to start creates the shared
+  memory on the host, subsequent QEMU processes will use it.
+
+- communication mechanism needed, an ivshmem server must be started before any
+  QEMU processes, then each QEMU process connects to the server unix socket.
+
+For more details on the QEMU ivshmem parameters, see qemu-doc documentation.
+
+
+Guest to guest communication
+
+
+This section details the communication mechanism between the guests accessing
+the ivhsmem shared memory.
 
-The device supports three BARs.  BAR0 is a 1 Kbyte MMIO region to support
-registers.  BAR1 is used for MSI-X when it is enabled in the device.  BAR2 is
-used to map the shared memory object from the host.  The size of BAR2 is
-specified when the guest is started and must be a power of 2 in size.
+*ivshmem server*
 
-*Registers*
+This server code is available in qemu.git/contrib/ivshmem-server.
 
-The device currently supports 4 registers of 32-bits each.  Registers
-are used for synchronization between guests sharing the same memory object when
-interrupts are supported (this requires using the shared memory server).
+The server must be started on the host before any guest.
+It creates a shared memory object then waits for clients to connect on an unix
+socket.
 
-The server assigns each VM an ID number and sends this ID number to the QEMU
-process when the guest starts.
+For each client (QEMU processes) that connects to the server:
+- 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,
+- the server creates a new set of host eventfds associated to the new client 
and
+  sends this set to all already connected clients,
+- finally, the server sends all the eventfds sets for all clients to the new
+  client.
+
+The server signals all clients when one of them disconnects.
+
+The client IDs are limited to 16 bits because of the current implementation 
(see
+Doorbell register in 'PCI device registers' subsection). Hence on 65536 clients
+are supported.
+
+All the file

Re: [PATCH 1/2] docs: update ivshmem device spec

2014-06-25 Thread David Marchand

Hello Claudio,

Sorry for the delay.
I am a bit short on time and will be offline for a week starting tonight.

I agree there are points that must be more clearly described (and I 
agree that ivshmem code will most likely have to be cleaned up after this).
Restructuring the documentation with a optional section is a good idea 
too.


I will work on this at my return.

Anyway, thanks for the review.


--
David Marchand


On 06/23/2014 04:18 PM, Claudio Fontana wrote:

Hi,

we were reading through this quickly today, and these are some of the questions 
that
we think can came up when reading this. Answers to some of these questions we 
think
we have figured out, but I think it's important to put this information into the
documentation.

I will quote the file in its entirety, and insert some questions inline.


Device Specification for Inter-VM shared memory device
--

The Inter-VM shared memory device is designed to share a region of memory to
userspace in multiple virtual guests.


What does to userspace mean in this context? The userspace of the host, or 
the userspace in the guest?

What about The Inter-VM shared memory device is designed to share a memory region 
(created on the host via the POSIX shared memory API) between multiple QEMU processes 
running different guests. In order for all guests to be able to pick up the shared memory 
area, it is modeled by QEMU as a PCI device exposing said memory to the guest as a PCI 
BAR.

Whether in those guests the memory region is used in kernel space or userspace, 
or there is even any meaning for those terms is guest-dependent I would think 
(I think of an OSv here, where the application and kernel execute at the same 
privilege level and in the same address space).


The memory region does not belong to any
guest, but is a POSIX memory object on the host.


Ok that's clear.
One thing I would ask is, but I don't know if it makes sense to mention here, 
is who creates this memory object on the host?
I understand in some cases it's the contributed server (what you provide in contrib/), in some 
cases it's the user of this device who has to write some server code for that, but 
is it true that also the qemu process itself can create this memory object on its own, without 
any external process needed? Is this the use case for host-guest only?


Optionally, the device may
support sending interrupts to other guests sharing the same memory region.


This opens a lot of questions here which are partly answered later (If I 
understand correctly, not only interrupts are involved, but a complete 
communication protocol involving registers in BAR0), but what about staying a 
bit general here, like
Optionally, the device may also provide a communication mechanism between 
guests sharing the same memory region. More details about that in the section 
'OPTIONAL ivshmem guest to guest communication protocol'.

Thinking out loud, I wonder if this communication mechanism should be part of 
this device in QEMU, or it should be provided at another layer..





The Inter-VM PCI device
---

*BARs*

The device supports three BARs.  BAR0 is a 1 Kbyte MMIO region to support
registers.  BAR1 is used for MSI-X when it is enabled in the device.  BAR2 is
used to map the shared memory object from the host.  The size of BAR2 is
specified when the guest is started and must be a power of 2 in size.


Are BAR0 and BAR1 optional? That's what I would think by reading the whole, but 
I'm still not sure.
Am I forced to map BAR0 and BAR1 anyway? I don't think so, but..

If so, can we separate the explanation into the base shared memory feature, and 
a separate section which explains the OPTIONAL communication mechanism, and the 
OPTIONAL MSI-X BAR?

For example, say that I am a potential ivshmem user (which I am), and I am 
interested in the shared memory but I want to use my own communication 
mechanism and protocol between guests, can we make it so that I don't have to 
wonder whether some of the info I read applies or not?
The solution to that I think is to put all the OPTIONAL parts into separate 
sections.



*Registers*


Ok, so this should I think go into one such OPTIONAL sections.



The device currently supports 4 registers of 32-bits each.  Registers
are used for synchronization between guests sharing the same memory object when
interrupts are supported (this requires using the shared memory server).


So use of BAR0 goes together with interrupts, and goes together with the shared 
memory server (is it the one contributed in contrib/?)



The server assigns each VM an ID number and sends this ID number to the QEMU
process when the guest starts.

enum ivshmem_registers {
 IntrMask = 0,
 IntrStatus = 4,
 IVPosition = 8,
 Doorbell = 12
};

The first two registers are the interrupt mask and status registers.  Mask and
status are only used with pin-based interrupts.  They are unused with MSI
interrupts

[PATCH 0/2] ivshmem: update documentation, add client/server tools

2014-06-20 Thread David Marchand
Hello, 

(as suggested by Paolo, ccing Claudio and kvm mailing list)

Here is a patchset containing an update on ivshmem specs documentation and
importing ivshmem server and client tools.
These tools have been written from scratch and are not related to what is
available in nahanni repository.
I put them in contrib/ directory as the qemu-doc.texi was already telling the
server was supposed to be there.

-- 
David Marchand

David Marchand (2):
  docs: update ivshmem device spec
  contrib: add ivshmem client and server

 contrib/ivshmem-client/Makefile |   26 ++
 contrib/ivshmem-client/ivshmem-client.c |  418 ++
 contrib/ivshmem-client/ivshmem-client.h |  238 ++
 contrib/ivshmem-client/main.c   |  246 ++
 contrib/ivshmem-server/Makefile |   26 ++
 contrib/ivshmem-server/ivshmem-server.c |  420 +++
 contrib/ivshmem-server/ivshmem-server.h |  185 ++
 contrib/ivshmem-server/main.c   |  296 ++
 docs/specs/ivshmem_device_spec.txt  |   41 ++-
 qemu-doc.texi   |   10 +-
 10 files changed, 1897 insertions(+), 9 deletions(-)
 create mode 100644 contrib/ivshmem-client/Makefile
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/Makefile
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] contrib: add ivshmem client and server

2014-06-20 Thread David Marchand
When using ivshmem devices, notifications between guests can be sent as
interrupts using a ivshmem-server (typical use described in documentation).
The client is provided as a debug tool.

Signed-off-by: David Marchand david.march...@6wind.com
---
 contrib/ivshmem-client/Makefile |   26 ++
 contrib/ivshmem-client/ivshmem-client.c |  418 ++
 contrib/ivshmem-client/ivshmem-client.h |  238 ++
 contrib/ivshmem-client/main.c   |  246 ++
 contrib/ivshmem-server/Makefile |   26 ++
 contrib/ivshmem-server/ivshmem-server.c |  420 +++
 contrib/ivshmem-server/ivshmem-server.h |  185 ++
 contrib/ivshmem-server/main.c   |  296 ++
 qemu-doc.texi   |   10 +-
 9 files changed, 1862 insertions(+), 3 deletions(-)
 create mode 100644 contrib/ivshmem-client/Makefile
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/Makefile
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c

diff --git a/contrib/ivshmem-client/Makefile b/contrib/ivshmem-client/Makefile
new file mode 100644
index 000..9e32409
--- /dev/null
+++ b/contrib/ivshmem-client/Makefile
@@ -0,0 +1,26 @@
+# Copyright 2014 6WIND S.A.
+# All rights reserved
+
+S ?= $(CURDIR)
+O ?= $(CURDIR)
+
+CFLAGS += -Wall -Wextra -Werror -g
+LDFLAGS +=
+LDLIBS += -lrt
+
+VPATH = $(S)
+PROG = ivshmem-client
+OBJS := $(O)/ivshmem-client.o
+OBJS += $(O)/main.o
+
+$(O)/%.o: %.c
+   $(CC) $(CFLAGS) -o $@ -c $
+
+$(O)/$(PROG): $(OBJS)
+   $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS)
+
+.PHONY: all
+all: $(O)/$(PROG)
+
+clean:
+   rm -f $(OBJS) $(O)/$(PROG)
diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
new file mode 100644
index 000..32ef3ef
--- /dev/null
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -0,0 +1,418 @@
+/*
+ * Copyright(c) 2014 6WIND S.A.
+ * All rights reserved.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include stdio.h
+#include stdint.h
+#include stdlib.h
+#include errno.h
+#include string.h
+#include signal.h
+#include unistd.h
+#include inttypes.h
+#include sys/queue.h
+
+#include sys/types.h
+#include sys/socket.h
+#include sys/un.h
+
+#include ivshmem-client.h
+
+/* log a message on stdout if verbose=1 */
+#define debug_log(client, fmt, ...) do { \
+if ((client)-verbose) { \
+printf(fmt, ## __VA_ARGS__); \
+}\
+} while (0)
+
+/* read message from the unix socket */
+static int
+read_one_msg(struct ivshmem_client *client, long *index, int *fd)
+{
+int ret;
+struct msghdr msg;
+struct iovec iov[1];
+union {
+struct cmsghdr cmsg;
+char control[CMSG_SPACE(sizeof(int))];
+} msg_control;
+struct cmsghdr *cmsg;
+
+iov[0].iov_base = index;
+iov[0].iov_len = sizeof(*index);
+
+memset(msg, 0, sizeof(msg));
+msg.msg_iov = iov;
+msg.msg_iovlen = 1;
+msg.msg_control = msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+ret = recvmsg(client-sock_fd, msg, 0);
+if (ret  0) {
+debug_log(client, cannot read message: %s\n, strerror(errno));
+return -1;
+}
+if (ret == 0) {
+debug_log(client, lost connection to server\n);
+return -1;
+}
+
+*fd = -1;
+
+for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+
+if (cmsg-cmsg_len != CMSG_LEN(sizeof(int)) ||
+cmsg-cmsg_level != SOL_SOCKET ||
+cmsg-cmsg_type != SCM_RIGHTS) {
+continue;
+}
+
+memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd));
+}
+
+return 0;
+}
+
+/* free a peer when the server advertise a disconnection or when the
+ * client is freed */
+static void
+free_peer(struct ivshmem_client *client, struct ivshmem_client_peer *peer)
+{
+unsigned vector;
+
+TAILQ_REMOVE(client-peer_list, peer, next);
+for (vector = 0; vector  peer-vectors_count; vector++) {
+close(peer-vectors[vector]);
+}
+
+free(peer);
+}
+
+/* handle message coming from server (new peer, new vectors) */
+static int
+handle_server_msg(struct ivshmem_client *client)
+{
+struct ivshmem_client_peer *peer;
+long peer_id;
+int ret, fd;
+
+ret = read_one_msg(client, peer_id, fd);
+if (ret  0) {
+return -1;
+}
+
+/* can return a peer or the local client */
+peer = ivshmem_client_search_peer(client, peer_id);
+
+/* delete peer */
+if (fd == -1) {
+
+if (peer == NULL || peer == client-local) {
+debug_log

[PATCH 1/2] docs: update ivshmem device spec

2014-06-20 Thread David Marchand
Add some notes on the parts needed to use ivshmem devices: more specifically,
explain the purpose of an ivshmem server and the basic concept to use the
ivshmem devices in guests.

Signed-off-by: David Marchand david.march...@6wind.com
---
 docs/specs/ivshmem_device_spec.txt |   41 ++--
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/docs/specs/ivshmem_device_spec.txt 
b/docs/specs/ivshmem_device_spec.txt
index 667a862..7d2b73f 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -85,12 +85,41 @@ events have occurred.  The semantics of interrupt vectors 
are left to the
 user's discretion.
 
 
+IVSHMEM host services
+-
+
+This part is optional (see *Usage in the Guest* section below).
+
+To handle notifications between users of a ivshmem device, a ivshmem server has
+been added. This server is responsible for creating the shared memory and
+creating a set of eventfds for each users of the shared memory. It behaves as a
+proxy between the different ivshmem clients (QEMU): giving the shared memory fd
+to each client, allocating eventfds to new clients and broadcasting to all
+clients when a client disappears.
+
+Apart from the current ivshmem implementation in QEMU, a ivshmem client can be
+written for debug, for development purposes, or to implement notifications
+between host and guests.
+
+
 Usage in the Guest
 --
 
-The shared memory device is intended to be used with the provided UIO driver.
-Very little configuration is needed.  The guest should map BAR0 to access the
-registers (an array of 32-bit ints allows simple writing) and map BAR2 to
-access the shared memory region itself.  The size of the shared memory region
-is specified when the guest (or shared memory server) is started.  A guest may
-map the whole shared memory region or only part of it.
+The guest should map BAR0 to access the registers (an array of 32-bit ints
+allows simple writing) and map BAR2 to access the shared memory region itself.
+The size of the shared memory region is specified when the guest (or shared
+memory server) is started.  A guest may map the whole shared memory region or
+only part of it.
+
+ivshmem provides an optional notification mechanism through eventfds handled by
+QEMU that will trigger interrupts in guests. This mechanism is enabled when
+using a ivshmem-server which must be started prior to VMs and which serves as a
+proxy for exchanging eventfds.
+
+It is your choice how to use the ivshmem device.
+- the simple way, you don't need anything else than what is already in QEMU.
+  You can map the shared memory in guest, then use it in userland as you see 
fit
+  (memnic for example works this way http://dpdk.org/browse/memnic),
+- the more advanced way, basically, if you want an event mechanism between the
+  VMs using your ivshmem device. In this case, then you will most likely want 
to
+  write a kernel driver that will handle interrupts.
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] Why I advise against using ivshmem

2014-06-19 Thread David Marchand

On 06/18/2014 05:01 PM, Andreas Färber wrote:

late onto this thread: SUSE Security team has just recently
done a thorough review of QEMU ivshmem code because a customer has
requested this be supported in SLES12. Multiple security-related
patches were submitted by Stefan Hajnoczi and Sebastian Krahmer, and I
fear they are probably still not merged for lack of active
maintainer... In such cases, after review, I expect them to be picked
up by Peter as committer or via qemu-trivial.

So -1, against dropping it.


Are these patches on patchwork ?


Vincent, you will find an RFC for an ivshmem-test in the qemu-devel
list archives or possibly on my qtest branch. The blocking issue that
I haven't worked on yet is that we can't unconditionally run the qtest
because it depends on KVM enabled at configure time (as opposed to
runtime) to have the device available.
http://patchwork.ozlabs.org/patch/336367/

As others have stated before, the nahanni server seems unmaintained,
thus not getting packaged by SUSE either and making testing the
interrupt parts of ivshmem difficult - unless we sort out and fill
with actual test code my proposed qtest.


Thanks for the RFC patch.

About ivshmem server, yes I will look at it.
I will see what I can propose or if importing nahanni implementation 
as-is is the best solution.


Anyway, first, documentation.


--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] Why I advise against using ivshmem

2014-06-18 Thread David Marchand

Hello Stefan,

On 06/18/2014 12:48 PM, Stefan Hajnoczi wrote:

One more thing to add to the list:

static void ivshmem_read(void *opaque, const uint8_t * buf, int flags)

The flags argument should be size.  Size should be checked before
accessing buf.


You are welcome to send a fix and I will review it.



Please also see the bug fixes in the following unapplied patch:
[PATCH] ivshmem: fix potential OOB r/w access (#2) by Sebastian Krahmer
https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg03538.html


Thanks for the pointer. I'll check it.



--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] Why I advise against using ivshmem

2014-06-18 Thread David Marchand


On 06/18/2014 12:51 PM, Stefan Hajnoczi wrote:


Actually, you can avoid this memory copy using frameworks like DPDK.


I guess it's careful to allocate all packets in the mmapped BAR?


Yes.


That's fine if you can modify applications but doesn't work for
unmodified applications using regular networking APIs.


If you have access to source code, this should not be a problem.



--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] Why I advise against using ivshmem

2014-06-17 Thread David Marchand

Hello all,

On 06/17/2014 04:54 AM, Stefan Hajnoczi wrote:

ivshmem has a performance disadvantage for guest-to-host
communication.  Since the shared memory is exposed as PCI BARs, the
guest has to memcpy into the shared memory.

vhost-user can access guest memory directly and avoid the copy inside the guest.


Actually, you can avoid this memory copy using frameworks like DPDK.



Unless someone steps up and maintains ivshmem, I think it should be
deprecated and dropped from QEMU.


Then I can maintain ivshmem for QEMU.
If this is ok, I will send a patch for MAINTAINERS file.


--
David Marchand
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html