From f981913abd9a5926d38a48c6ba9b1ba7dacb1c11 Mon Sep 17 00:00:00 2001 From: Mattias Nissler <122288598+mnissler-rivos@users.noreply.github.com> Date: Wed, 30 Aug 2023 17:19:50 +0200 Subject: [PATCH] Replace protocol header flags bit field with mask (#773) It turns out that the bit field will not yield the desired / specified bit layout on big-endian systems, see issue #768 for details. Thus, replace the bit field with constants for the individual fields and use bit masking when accessing the flags field. Signed-off-by: Mattias Nissler Reviewed-by: John Levon Reviewed-by: Thanos Makatos --- include/vfio-user.h | 14 ++++++-------- lib/libvfio-user.c | 4 ++-- lib/tran_pipe.c | 12 ++++++------ lib/tran_sock.c | 12 ++++++------ test/unit-tests.c | 2 +- 5 files changed, 21 insertions(+), 23 deletions(-) diff --git a/include/vfio-user.h b/include/vfio-user.h index 52f08707..a749938c 100644 --- a/include/vfio-user.h +++ b/include/vfio-user.h @@ -81,14 +81,12 @@ struct vfio_user_header { uint16_t msg_id; uint16_t cmd; uint32_t msg_size; - struct { - uint32_t type : 4; -#define VFIO_USER_F_TYPE_COMMAND 0 -#define VFIO_USER_F_TYPE_REPLY 1 - uint32_t no_reply : 1; - uint32_t error : 1; - uint32_t resvd : 26; - } flags; + uint32_t flags; +#define VFIO_USER_F_TYPE_MASK (0xf) +#define VFIO_USER_F_TYPE_COMMAND (0x0) +#define VFIO_USER_F_TYPE_REPLY (0x1) +#define VFIO_USER_F_NO_REPLY (0x10) +#define VFIO_USER_F_ERROR (0x20) uint32_t error_no; } __attribute__((packed)); diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index 48013c28..271a269a 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -1127,7 +1127,7 @@ do_reply(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg, int reply_errno) assert(vfu_ctx != NULL); assert(msg != NULL); - if (msg->hdr.flags.no_reply) { + if (msg->hdr.flags & VFIO_USER_F_NO_REPLY) { /* * A failed client request is not a failure of handle_request() itself. */ @@ -1283,7 +1283,7 @@ get_request_header(vfu_ctx_t *vfu_ctx, vfu_msg_t **msgp) static bool is_valid_header(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg) { - if (msg->hdr.flags.type != VFIO_USER_F_TYPE_COMMAND) { + if ((msg->hdr.flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_COMMAND) { vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: not a command req", msg->hdr.msg_id); return false; diff --git a/lib/tran_pipe.c b/lib/tran_pipe.c index d1428abf..e7aa84d0 100644 --- a/lib/tran_pipe.c +++ b/lib/tran_pipe.c @@ -57,15 +57,15 @@ tran_pipe_send_iovec(int fd, uint16_t msg_id, bool is_reply, } if (is_reply) { - hdr.flags.type = VFIO_USER_F_TYPE_REPLY; + hdr.flags |= VFIO_USER_F_TYPE_REPLY; hdr.cmd = cmd; if (err != 0) { - hdr.flags.error = 1U; + hdr.flags |= VFIO_USER_F_ERROR; hdr.error_no = err; } } else { hdr.cmd = cmd; - hdr.flags.type = VFIO_USER_F_TYPE_COMMAND; + hdr.flags |= VFIO_USER_F_TYPE_COMMAND; } iovecs[0].iov_base = &hdr; @@ -131,18 +131,18 @@ tran_pipe_recv(int fd, struct vfio_user_header *hdr, bool is_reply, return ERROR_INT(EPROTO); } - if (hdr->flags.type != VFIO_USER_F_TYPE_REPLY) { + if ((hdr->flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_REPLY) { return ERROR_INT(EINVAL); } - if (hdr->flags.error == 1U) { + if (hdr->flags & VFIO_USER_F_ERROR) { if (hdr->error_no <= 0) { hdr->error_no = EINVAL; } return ERROR_INT(hdr->error_no); } } else { - if (hdr->flags.type != VFIO_USER_F_TYPE_COMMAND) { + if ((hdr->flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_COMMAND) { return ERROR_INT(EINVAL); } if (msg_id != NULL) { diff --git a/lib/tran_sock.c b/lib/tran_sock.c index fb0ccbb6..3f4c8c38 100644 --- a/lib/tran_sock.c +++ b/lib/tran_sock.c @@ -69,15 +69,15 @@ tran_sock_send_iovec(int sock, uint16_t msg_id, bool is_reply, memset(&msg, 0, sizeof(msg)); if (is_reply) { - hdr.flags.type = VFIO_USER_F_TYPE_REPLY; + hdr.flags |= VFIO_USER_F_TYPE_REPLY; hdr.cmd = cmd; if (err != 0) { - hdr.flags.error = 1U; + hdr.flags |= VFIO_USER_F_ERROR; hdr.error_no = err; } } else { hdr.cmd = cmd; - hdr.flags.type = VFIO_USER_F_TYPE_COMMAND; + hdr.flags |= VFIO_USER_F_TYPE_COMMAND; } iovecs[0].iov_base = &hdr; @@ -211,18 +211,18 @@ tran_sock_recv_fds(int sock, struct vfio_user_header *hdr, bool is_reply, return ERROR_INT(EPROTO); } - if (hdr->flags.type != VFIO_USER_F_TYPE_REPLY) { + if ((hdr->flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_REPLY) { return ERROR_INT(EINVAL); } - if (hdr->flags.error == 1U) { + if (hdr->flags & VFIO_USER_F_ERROR) { if (hdr->error_no <= 0) { hdr->error_no = EINVAL; } return ERROR_INT(hdr->error_no); } } else { - if (hdr->flags.type != VFIO_USER_F_TYPE_COMMAND) { + if ((hdr->flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_COMMAND) { return ERROR_INT(EINVAL); } if (msg_id != NULL) { diff --git a/test/unit-tests.c b/test/unit-tests.c index 0fd4fe77..310eb238 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -105,7 +105,7 @@ setup(void **state UNUSED) memset(&msg, 0, sizeof(msg)); - msg.hdr.flags.type = VFIO_USER_F_TYPE_COMMAND; + msg.hdr.flags |= VFIO_USER_F_TYPE_COMMAND; msg.hdr.msg_size = sizeof(msg.hdr); fds[0] = fds[1] = -1;