ivshmem: use little-endian int64_t for the protocol The current ivshmem protocol uses 'long' for integers. But the sizeof(long) depends on the host and the endianess is not defined, which may cause portability troubles. Instead, switch to using little-endian int64_t. This breaks the protocol, except on x64 little-endian host where this change should be compatible. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
diff --git a/contrib/ivshmem-client/ivshmem-client.c b/contrib/ivshmem-client/ivshmem-client.c index 076e3ec..31619d8 100644 --- a/contrib/ivshmem-client/ivshmem-client.c +++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -24,7 +24,7 @@ /* read message from the unix socket */ static int -ivshmem_client_read_one_msg(IvshmemClient *client, long *index, int *fd) +ivshmem_client_read_one_msg(IvshmemClient *client, int64_t *index, int *fd) { int ret; struct msghdr msg; @@ -45,7 +45,7 @@ msg.msg_controllen = sizeof(msg_control); ret = recvmsg(client->sock_fd, &msg, 0); - if (ret < 0) { + if (ret < sizeof(*index)) { IVSHMEM_CLIENT_DEBUG(client, "cannot read message: %s\n", strerror(errno)); return -1; @@ -55,6 +55,7 @@ return -1; } + *index = GINT64_FROM_LE(*index); *fd = -1; for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) { @@ -91,7 +92,7 @@ ivshmem_client_handle_server_msg(IvshmemClient *client) { IvshmemClientPeer *peer; - long peer_id; + int64_t peer_id; int ret, fd; ret = ivshmem_client_read_one_msg(client, &peer_id, &fd); @@ -107,11 +108,11 @@ if (peer == NULL || peer == &client->local) { IVSHMEM_CLIENT_DEBUG(client, "receive delete for invalid " - "peer %ld\n", peer_id); + "peer %" PRId64 "\n", peer_id); return -1; } - IVSHMEM_CLIENT_DEBUG(client, "delete peer id = %ld\n", peer_id); + IVSHMEM_CLIENT_DEBUG(client, "delete peer id = %" PRId64 "\n", peer_id); ivshmem_client_free_peer(client, peer); return 0; } @@ -122,12 +123,12 @@ peer->id = peer_id; peer->vectors_count = 0; QTAILQ_INSERT_TAIL(&client->peer_list, peer, next); - IVSHMEM_CLIENT_DEBUG(client, "new peer id = %ld\n", peer_id); + IVSHMEM_CLIENT_DEBUG(client, "new peer id = %" PRId64 "\n", peer_id); } /* new vector */ - IVSHMEM_CLIENT_DEBUG(client, " new vector %d (fd=%d) for peer id %ld\n", - peer->vectors_count, fd, peer->id); + IVSHMEM_CLIENT_DEBUG(client, " new vector %d (fd=%d) for peer id %" + PRId64 "\n", peer->vectors_count, fd, peer->id); if (peer->vectors_count >= G_N_ELEMENTS(peer->vectors)) { IVSHMEM_CLIENT_DEBUG(client, "Too many vectors received, failing"); return -1; @@ -180,7 +181,7 @@ { struct sockaddr_un sun; int fd, ret; - long tmp; + int64_t tmp; IVSHMEM_CLIENT_DEBUG(client, "connect to client %s\n", client->unix_sock_path); @@ -219,7 +220,7 @@ IVSHMEM_CLIENT_DEBUG(client, "cannot read from server (2)\n"); goto err_close; } - IVSHMEM_CLIENT_DEBUG(client, "our_id=%ld\n", client->local.id); + IVSHMEM_CLIENT_DEBUG(client, "our_id=%" PRId64 "\n", client->local.id); /* now, we expect shared mem fd + a -1 index, note that shm fd * is not used */ @@ -350,13 +351,13 @@ int fd; if (vector >= peer->vectors_count) { - IVSHMEM_CLIENT_DEBUG(client, "invalid vector %u on peer %ld\n", + IVSHMEM_CLIENT_DEBUG(client, "invalid vector %u on peer %" PRId64 "\n", vector, peer->id); return -1; } fd = peer->vectors[vector]; - IVSHMEM_CLIENT_DEBUG(client, "notify peer %ld on vector %d, fd %d\n", - peer->id, vector, fd); + IVSHMEM_CLIENT_DEBUG(client, "notify peer %" PRId64 + " on vector %d, fd %d\n", peer->id, vector, fd); kick = 1; if (write(fd, &kick, sizeof(kick)) != sizeof(kick)) { @@ -402,7 +403,7 @@ /* lookup peer from its id */ IvshmemClientPeer * -ivshmem_client_search_peer(IvshmemClient *client, long peer_id) +ivshmem_client_search_peer(IvshmemClient *client, int64_t peer_id) { IvshmemClientPeer *peer; @@ -427,7 +428,7 @@ /* dump local infos */ peer = &client->local; - printf("our_id = %ld\n", peer->id); + printf("our_id = %" PRId64 "\n", peer->id); for (vector = 0; vector < peer->vectors_count; vector++) { printf(" vector %d is enabled (fd=%d)\n", vector, peer->vectors[vector]); @@ -435,7 +436,7 @@ /* dump peers */ QTAILQ_FOREACH(peer, &client->peer_list, next) { - printf("peer_id = %ld\n", peer->id); + printf("peer_id = %" PRId64 "\n", peer->id); for (vector = 0; vector < peer->vectors_count; vector++) { printf(" vector %d is enabled (fd=%d)\n", vector,
diff --git a/contrib/ivshmem-client/ivshmem-client.h b/contrib/ivshmem-client/ivshmem-client.h index 9215f34..3a4f809 100644 --- a/contrib/ivshmem-client/ivshmem-client.h +++ b/contrib/ivshmem-client/ivshmem-client.h
@@ -43,7 +43,7 @@ */ typedef struct IvshmemClientPeer { QTAILQ_ENTRY(IvshmemClientPeer) next; /**< next in list*/ - long id; /**< the id of the peer */ + int64_t id; /**< the id of the peer */ int vectors[IVSHMEM_CLIENT_MAX_VECTORS]; /**< one fd per vector */ unsigned vectors_count; /**< number of vectors */ } IvshmemClientPeer; @@ -198,7 +198,7 @@ * Returns: The peer structure, or NULL if not found */ IvshmemClientPeer * -ivshmem_client_search_peer(IvshmemClient *client, long peer_id); +ivshmem_client_search_peer(IvshmemClient *client, int64_t peer_id); /** * Dump information of this ivshmem client on stdout
diff --git a/contrib/ivshmem-client/main.c b/contrib/ivshmem-client/main.c index 28dd81e..c004870 100644 --- a/contrib/ivshmem-client/main.c +++ b/contrib/ivshmem-client/main.c
@@ -179,7 +179,8 @@ { (void)client; (void)arg; - printf("receive notification from peer_id=%ld vector=%d\n", peer->id, vect); + printf("receive notification from peer_id=%" PRId64 " vector=%u\n", + peer->id, vect); } int
diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c index e8693de..5e5239c 100644 --- a/contrib/ivshmem-server/ivshmem-server.c +++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -33,7 +33,7 @@ /* send message to a client unix socket */ static int -ivshmem_server_send_one_msg(int sock_fd, long peer_id, int fd) +ivshmem_server_send_one_msg(int sock_fd, int64_t peer_id, int fd) { int ret; struct msghdr msg; @@ -44,6 +44,7 @@ } msg_control; struct cmsghdr *cmsg; + peer_id = GINT64_TO_LE(peer_id); iov[0].iov_base = &peer_id; iov[0].iov_len = sizeof(peer_id); @@ -79,7 +80,7 @@ unsigned vector; IvshmemServerPeer *other_peer; - IVSHMEM_SERVER_DEBUG(server, "free peer %ld\n", peer->id); + IVSHMEM_SERVER_DEBUG(server, "free peer %" PRId64 "\n", peer->id); close(peer->sock_fd); QTAILQ_REMOVE(&server->peer_list, peer, next); @@ -209,7 +210,7 @@ } QTAILQ_INSERT_TAIL(&server->peer_list, peer, next); - IVSHMEM_SERVER_DEBUG(server, "new peer id = %ld\n", + IVSHMEM_SERVER_DEBUG(server, "new peer id = %" PRId64 "\n", peer->id); return 0; @@ -459,7 +460,7 @@ /* lookup peer from its id */ IvshmemServerPeer * -ivshmem_server_search_peer(IvshmemServer *server, long peer_id) +ivshmem_server_search_peer(IvshmemServer *server, int64_t peer_id) { IvshmemServerPeer *peer; @@ -480,7 +481,7 @@ /* dump peers */ QTAILQ_FOREACH(peer, &server->peer_list, next) { - printf("peer_id = %ld\n", peer->id); + printf("peer_id = %" PRId64 "\n", peer->id); for (vector = 0; vector < peer->vectors_count; vector++) { printf(" vector %d is enabled (fd=%d)\n", vector,
diff --git a/contrib/ivshmem-server/ivshmem-server.h b/contrib/ivshmem-server/ivshmem-server.h index 89c905f..c9359a0 100644 --- a/contrib/ivshmem-server/ivshmem-server.h +++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -51,7 +51,7 @@ typedef struct IvshmemServerPeer { QTAILQ_ENTRY(IvshmemServerPeer) next; /**< next in list*/ int sock_fd; /**< connected unix sock */ - long id; /**< the id of the peer */ + int64_t id; /**< the id of the peer */ EventNotifier vectors[IVSHMEM_SERVER_MAX_VECTORS]; /**< one per vector */ unsigned vectors_count; /**< number of vectors */ } IvshmemServerPeer; @@ -155,7 +155,7 @@ * Returns: The peer structure, or NULL if not found */ IvshmemServerPeer * -ivshmem_server_search_peer(IvshmemServer *server, long peer_id); +ivshmem_server_search_peer(IvshmemServer *server, int64_t peer_id); /** * Dump information of this ivshmem server and its peers on stdout
diff --git a/docs/specs/ivshmem_device_spec.txt b/docs/specs/ivshmem_device_spec.txt index 3435116..d318d65 100644 --- a/docs/specs/ivshmem_device_spec.txt +++ b/docs/specs/ivshmem_device_spec.txt
@@ -61,7 +61,7 @@ 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. +socket. All the messages are little-endian int64_t integer. For each client (QEMU process) that connects to the server: - the server sends a protocol version, if client does not support it, the client
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index e9dae83..83d7bd3 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c
@@ -276,7 +276,7 @@ static int ivshmem_can_receive(void * opaque) { - return sizeof(long); + return sizeof(int64_t); } static void ivshmem_event(void *opaque, int event) @@ -516,7 +516,7 @@ const uint8_t *p; uint32_t num; - assert(len <= sizeof(long)); /* limitation of the fifo */ + assert(len <= sizeof(int64_t)); /* limitation of the fifo */ if (fifo8_is_empty(&s->incoming_fifo) && size == len) { memcpy(data, buf, size); return true; @@ -524,7 +524,7 @@ IVSHMEM_DPRINTF("short read of %d bytes\n", size); - num = MIN(size, sizeof(long) - fifo8_num_used(&s->incoming_fifo)); + num = MIN(size, sizeof(int64_t) - fifo8_num_used(&s->incoming_fifo)); fifo8_push_all(&s->incoming_fifo, buf, num); if (fifo8_num_used(&s->incoming_fifo) < len) { @@ -546,6 +546,17 @@ return true; } +static bool fifo_update_and_get_i64(IVShmemState *s, + const uint8_t *buf, int size, int64_t *i64) +{ + if (fifo_update_and_get(s, buf, size, i64, sizeof(*i64))) { + *i64 = GINT64_FROM_LE(*i64); + return true; + } + + return false; +} + static int ivshmem_add_kvm_msi_virq(IVShmemState *s, int vector) { PCIDevice *pdev = PCI_DEVICE(s); @@ -603,23 +614,23 @@ IVShmemState *s = opaque; int incoming_fd; int new_eventfd; - long incoming_posn; + int64_t incoming_posn; Error *err = NULL; Peer *peer; - if (!fifo_update_and_get(s, buf, size, - &incoming_posn, sizeof(incoming_posn))) { + if (!fifo_update_and_get_i64(s, buf, size, &incoming_posn)) { return; } if (incoming_posn < -1) { - IVSHMEM_DPRINTF("invalid incoming_posn %ld\n", incoming_posn); + IVSHMEM_DPRINTF("invalid incoming_posn %" PRId64 "\n", incoming_posn); return; } /* pick off s->server_chr->msgfd and store it, posn should accompany msg */ incoming_fd = qemu_chr_fe_get_msgfd(s->server_chr); - IVSHMEM_DPRINTF("posn is %ld, fd is %d\n", incoming_posn, incoming_fd); + IVSHMEM_DPRINTF("posn is %" PRId64 ", fd is %d\n", + incoming_posn, incoming_fd); /* make sure we have enough space for this peer */ if (incoming_posn >= s->nb_peers) { @@ -641,7 +652,7 @@ s->vm_id = incoming_posn; } else { /* otherwise an fd == -1 means an existing peer has gone away */ - IVSHMEM_DPRINTF("posn %ld has gone away\n", incoming_posn); + IVSHMEM_DPRINTF("posn %" PRId64 " has gone away\n", incoming_posn); close_peer_eventfds(s, incoming_posn); } return; @@ -697,7 +708,7 @@ new_eventfd = peer->nb_eventfds++; /* this is an eventfd for a particular peer VM */ - IVSHMEM_DPRINTF("eventfds[%ld][%d] = %d\n", incoming_posn, + IVSHMEM_DPRINTF("eventfds[%" PRId64 "][%d] = %d\n", incoming_posn, new_eventfd, incoming_fd); event_notifier_init_fd(&peer->eventfds[new_eventfd], incoming_fd); fcntl_setfl(incoming_fd, O_NONBLOCK); /* msix/irqfd poll non block */ @@ -715,10 +726,9 @@ { IVShmemState *s = opaque; int tmp; - long version; + int64_t version; - if (!fifo_update_and_get(s, buf, size, - &version, sizeof(version))) { + if (!fifo_update_and_get_i64(s, buf, size, &version)) { return; } @@ -873,7 +883,7 @@ s->ivshmem_size = size; } - fifo8_create(&s->incoming_fifo, sizeof(long)); + fifo8_create(&s->incoming_fifo, sizeof(int64_t)); /* IRQFD requires MSI */ if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) &&