[PATCH v4 08/12] libvhost-user: enable it on any POSIX system

Stefano Garzarella posted 12 patches 6 months, 3 weeks ago
Maintainers: David Hildenbrand <david@redhat.com>, Igor Mammedov <imammedo@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, Gerd Hoffmann <kraxel@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Jason Wang <jasowang@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Coiby Xu <Coiby.Xu@gmail.com>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH v4 08/12] libvhost-user: enable it on any POSIX system
Posted by Stefano Garzarella 6 months, 3 weeks ago
The vhost-user protocol is not really Linux-specific so let's enable
libvhost-user for any POSIX system.

Compiling it on macOS and FreeBSD some problems came up:
- avoid to include linux/vhost.h which is avaibale only on Linux
  (vhost_types.h contains many of the things we need)
- macOS doesn't provide sys/endian.h, so let's define them
  (note: libvhost-user doesn't include qemu's headers, so we can't use
   use "qemu/bswap.h")
- define eventfd_[write|read] as write/read wrapper when system doesn't
  provide those (e.g. macOS)
- copy SEAL defines from include/qemu/memfd.h to make the code works
  on FreeBSD where MFD_ALLOW_SEALING is defined
- define MAP_NORESERVE if it's not defined (e.g. on FreeBSD)

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 meson.build                               |  2 +-
 subprojects/libvhost-user/libvhost-user.h |  2 +-
 subprojects/libvhost-user/libvhost-user.c | 60 +++++++++++++++++++++--
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index 955921dcb8..7954da5971 100644
--- a/meson.build
+++ b/meson.build
@@ -3168,7 +3168,7 @@ endif
 config_host_data.set('CONFIG_FDT', fdt.found())
 
 vhost_user = not_found
-if host_os == 'linux' and have_vhost_user
+if have_vhost_user
   libvhost_user = subproject('libvhost-user')
   vhost_user = libvhost_user.get_variable('vhost_user_dep')
 endif
diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index deb40e77b3..e13e1d3931 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -18,9 +18,9 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <poll.h>
-#include <linux/vhost.h>
 #include <pthread.h>
 #include "standard-headers/linux/virtio_ring.h"
+#include "standard-headers/linux/vhost_types.h"
 
 /* Based on qemu/hw/virtio/vhost-user.c */
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 1c361ffd51..03edb4bf64 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -28,9 +28,7 @@
 #include <inttypes.h>
 #include <sys/types.h>
 #include <sys/socket.h>
-#include <sys/eventfd.h>
 #include <sys/mman.h>
-#include <endian.h>
 
 /* Necessary to provide VIRTIO_F_VERSION_1 on system
  * with older linux headers. Must appear before
@@ -39,8 +37,8 @@
 #include "standard-headers/linux/virtio_config.h"
 
 #if defined(__linux__)
+#include <endian.h>
 #include <sys/syscall.h>
-#include <fcntl.h>
 #include <sys/ioctl.h>
 #include <linux/vhost.h>
 #include <sys/vfs.h>
@@ -52,6 +50,62 @@
 
 #endif
 
+#if defined(__APPLE__) && (__MACH__)
+#include <libkern/OSByteOrder.h>
+#define htobe16(x) OSSwapHostToBigInt16(x)
+#define htole16(x) OSSwapHostToLittleInt16(x)
+#define be16toh(x) OSSwapBigToHostInt16(x)
+#define le16toh(x) OSSwapLittleToHostInt16(x)
+
+#define htobe32(x) OSSwapHostToBigInt32(x)
+#define htole32(x) OSSwapHostToLittleInt32(x)
+#define be32toh(x) OSSwapBigToHostInt32(x)
+#define le32toh(x) OSSwapLittleToHostInt32(x)
+
+#define htobe64(x) OSSwapHostToBigInt64(x)
+#define htole64(x) OSSwapHostToLittleInt64(x)
+#define be64toh(x) OSSwapBigToHostInt64(x)
+#define le64toh(x) OSSwapLittleToHostInt64(x)
+#endif
+
+#ifdef CONFIG_EVENTFD
+#include <sys/eventfd.h>
+#else
+#define eventfd_t uint64_t
+
+int eventfd_write(int fd, eventfd_t value)
+{
+    return (write(fd, &value, sizeof(value)) == sizeof(value)) ? 0 : -1;
+}
+
+int eventfd_read(int fd, eventfd_t *value)
+{
+    return (read(fd, value, sizeof(*value)) == sizeof(*value)) ? 0 : -1;
+}
+#endif
+
+#ifdef MFD_ALLOW_SEALING
+#include <fcntl.h>
+
+#ifndef F_LINUX_SPECIFIC_BASE
+#define F_LINUX_SPECIFIC_BASE 1024
+#endif
+
+#ifndef F_ADD_SEALS
+#define F_ADD_SEALS (F_LINUX_SPECIFIC_BASE + 9)
+#define F_GET_SEALS (F_LINUX_SPECIFIC_BASE + 10)
+
+#define F_SEAL_SEAL     0x0001  /* prevent further seals from being set */
+#define F_SEAL_SHRINK   0x0002  /* prevent file from shrinking */
+#define F_SEAL_GROW     0x0004  /* prevent file from growing */
+#define F_SEAL_WRITE    0x0008  /* prevent writes */
+#endif
+#endif
+
+#ifndef MAP_NORESERVE
+#define MAP_NORESERVE 0
+#endif
+
 #include "include/atomic.h"
 
 #include "libvhost-user.h"
-- 
2.45.0
Re: [PATCH v4 08/12] libvhost-user: enable it on any POSIX system
Posted by Philippe Mathieu-Daudé 6 months, 3 weeks ago
On 8/5/24 09:44, Stefano Garzarella wrote:
> The vhost-user protocol is not really Linux-specific so let's enable
> libvhost-user for any POSIX system.
> 
> Compiling it on macOS and FreeBSD some problems came up:
> - avoid to include linux/vhost.h which is avaibale only on Linux

"available"

>    (vhost_types.h contains many of the things we need)
> - macOS doesn't provide sys/endian.h, so let's define them
>    (note: libvhost-user doesn't include qemu's headers, so we can't use

"QEMU"

>     use "qemu/bswap.h")
> - define eventfd_[write|read] as write/read wrapper when system doesn't
>    provide those (e.g. macOS)
> - copy SEAL defines from include/qemu/memfd.h to make the code works
>    on FreeBSD where MFD_ALLOW_SEALING is defined

Alternatively add in subprojects/libvhost-user/include/osdep.h.

> - define MAP_NORESERVE if it's not defined (e.g. on FreeBSD)
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   meson.build                               |  2 +-
>   subprojects/libvhost-user/libvhost-user.h |  2 +-
>   subprojects/libvhost-user/libvhost-user.c | 60 +++++++++++++++++++++--
>   3 files changed, 59 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>



Re: [PATCH v4 08/12] libvhost-user: enable it on any POSIX system
Posted by Stefano Garzarella 6 months, 2 weeks ago
On Wed, May 08, 2024 at 12:36:30PM GMT, Philippe Mathieu-Daudé wrote:
>On 8/5/24 09:44, Stefano Garzarella wrote:
>>The vhost-user protocol is not really Linux-specific so let's enable
>>libvhost-user for any POSIX system.
>>
>>Compiling it on macOS and FreeBSD some problems came up:
>>- avoid to include linux/vhost.h which is avaibale only on Linux
>
>"available"
>
>>   (vhost_types.h contains many of the things we need)
>>- macOS doesn't provide sys/endian.h, so let's define them
>>   (note: libvhost-user doesn't include qemu's headers, so we can't use
>
>"QEMU"
>

Good catches, I'll fix them!

>>    use "qemu/bswap.h")
>>- define eventfd_[write|read] as write/read wrapper when system doesn't
>>   provide those (e.g. macOS)
>>- copy SEAL defines from include/qemu/memfd.h to make the code works
>>   on FreeBSD where MFD_ALLOW_SEALING is defined
>
>Alternatively add in subprojects/libvhost-user/include/osdep.h.

I like the idea, but we also have other things already present before
this patch (e.g. G_GNUC_PRINTF, MIN, etc.) so do you think it's better
to add 2 patches (move everything to osdep.h, add things from this
patch), or after this series is merged, send a patch to introduce
osdep.h?

I'm tempted for the last option just to prevent this series from
becoming too big, but I don't have a strong opinion.

Thanks,
Stefano

>
>>- define MAP_NORESERVE if it's not defined (e.g. on FreeBSD)
>>
>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>---
>>  meson.build                               |  2 +-
>>  subprojects/libvhost-user/libvhost-user.h |  2 +-
>>  subprojects/libvhost-user/libvhost-user.c | 60 +++++++++++++++++++++--
>>  3 files changed, 59 insertions(+), 5 deletions(-)
>
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
Re: [PATCH v4 08/12] libvhost-user: enable it on any POSIX system
Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago
On 10/5/24 10:56, Stefano Garzarella wrote:
> On Wed, May 08, 2024 at 12:36:30PM GMT, Philippe Mathieu-Daudé wrote:
>> On 8/5/24 09:44, Stefano Garzarella wrote:
>>> The vhost-user protocol is not really Linux-specific so let's enable
>>> libvhost-user for any POSIX system.


>> Alternatively add in subprojects/libvhost-user/include/osdep.h.
> 
> I like the idea, but we also have other things already present before
> this patch (e.g. G_GNUC_PRINTF, MIN, etc.) so do you think it's better
> to add 2 patches (move everything to osdep.h, add things from this
> patch), or after this series is merged, send a patch to introduce
> osdep.h?
> 
> I'm tempted for the last option just to prevent this series from
> becoming too big, but I don't have a strong opinion.

Whichever is simpler for your workflow works for me :)

Regards,

Phil.