| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACGkMEu28fHr7Bo5Zm4chwOj-xBmTYcHM3TfXRx8OZ3OhO8q8Q@mail.gmail.com>
Date: Wed, 19 Nov 2025 10:03:58 +0800
From: Jason Wang <jasowang@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: linux-kernel@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
Paolo Abeni <pabeni@...hat.com>, Eugenio Pérez <eperezma@...hat.com>,
Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Jonathan Corbet <corbet@....net>, kvm@...r.kernel.org,
virtualization@...ts.linux.dev, netdev@...r.kernel.org,
linux-doc@...r.kernel.org, Mike Christie <michael.christie@...cle.com>,
Paolo Bonzini <pbonzini@...hat.com>, Stefan Hajnoczi <stefanha@...hat.com>,
Stefano Garzarella <sgarzare@...hat.com>
Subject: Re: [PATCH v4 2/2] vhost: switch to arrays of feature bits
On Sun, Nov 16, 2025 at 3:45 PM Michael S. Tsirkin <mst@...hat.com> wrote:
>
> The current interface where caller has to know in which 64 bit chunk
> each bit is, is inelegant and fragile.
> Let's simply use arrays of bits.
> By using unroll macros text size grows only slightly.
>
> Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> ---
> drivers/vhost/net.c | 34 +++++++++++++++++++---------------
> drivers/vhost/scsi.c | 9 ++++++---
> drivers/vhost/test.c | 10 ++++++++--
> drivers/vhost/vhost.h | 42 ++++++++++++++++++++++++++++++++++--------
> drivers/vhost/vsock.c | 10 ++++++----
> 5 files changed, 73 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index d057ea55f5ad..00d00034a97e 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -69,15 +69,15 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
>
> #define VHOST_DMA_IS_DONE(len) ((__force u32)(len) >= (__force u32)VHOST_DMA_DONE_LEN)
>
> -static const u64 vhost_net_features[VIRTIO_FEATURES_U64S] = {
> - VHOST_FEATURES |
> - (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
> - (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
> - (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
> - (1ULL << VIRTIO_F_RING_RESET) |
> - (1ULL << VIRTIO_F_IN_ORDER),
> - VIRTIO_BIT(VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) |
> - VIRTIO_BIT(VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO),
> +static const int vhost_net_features[] = {
> + VHOST_FEATURES,
> + VHOST_NET_F_VIRTIO_NET_HDR,
> + VIRTIO_NET_F_MRG_RXBUF,
> + VIRTIO_F_ACCESS_PLATFORM,
> + VIRTIO_F_RING_RESET,
> + VIRTIO_F_IN_ORDER,
> + VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO,
> + VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO
> };
>
> enum {
> @@ -1734,14 +1734,14 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
> return -EFAULT;
> return vhost_net_set_backend(n, backend.index, backend.fd);
> case VHOST_GET_FEATURES:
> - features = vhost_net_features[0];
> + features = VHOST_FEATURES_U64(vhost_net_features, 0);
> if (copy_to_user(featurep, &features, sizeof features))
> return -EFAULT;
> return 0;
> case VHOST_SET_FEATURES:
> if (copy_from_user(&features, featurep, sizeof features))
> return -EFAULT;
> - if (features & ~vhost_net_features[0])
> + if (features & ~VHOST_FEATURES_U64(vhost_net_features, 0))
> return -EOPNOTSUPP;
>
> virtio_features_from_u64(all_features, features);
> @@ -1753,9 +1753,13 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
> /* Copy the net features, up to the user-provided buffer size */
> argp += sizeof(u64);
> copied = min(count, (u64)VIRTIO_FEATURES_U64S);
> - if (copy_to_user(argp, vhost_net_features,
> - copied * sizeof(u64)))
> - return -EFAULT;
> +
> + {
> + const DEFINE_VHOST_FEATURES_ARRAY(features, vhost_net_features);
> +
> + if (copy_to_user(argp, features, copied * sizeof(u64)))
> + return -EFAULT;
> + }
Any reason to use a standalone block here?
>
> /* Zero the trailing space provided by user-space, if any */
> if (clear_user(argp, size_mul(count - copied, sizeof(u64))))
> @@ -1784,7 +1788,7 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
> }
>
> for (i = 0; i < VIRTIO_FEATURES_U64S; i++)
> - if (all_features[i] & ~vhost_net_features[i])
> + if (all_features[i] & ~VHOST_FEATURES_U64(vhost_net_features, i))
> return -EOPNOTSUPP;
>
> return vhost_net_set_features(n, all_features);
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 98e4f68f4e3c..04fcbe7efd77 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -197,11 +197,14 @@ enum {
> };
>
> /* Note: can't set VIRTIO_F_VERSION_1 yet, since that implies ANY_LAYOUT. */
> -enum {
> - VHOST_SCSI_FEATURES = VHOST_FEATURES | (1ULL << VIRTIO_SCSI_F_HOTPLUG) |
> - (1ULL << VIRTIO_SCSI_F_T10_PI)
> +static const int vhost_scsi_features[] = {
> + VHOST_FEATURES,
> + VIRTIO_SCSI_F_HOTPLUG,
> + VIRTIO_SCSI_F_T10_PI
> };
>
> +#define VHOST_SCSI_FEATURES VHOST_FEATURES_U64(vhost_scsi_features, 0)
> +
> #define VHOST_SCSI_MAX_TARGET 256
> #define VHOST_SCSI_MAX_IO_VQ 1024
> #define VHOST_SCSI_MAX_EVENT 128
> diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> index 42c955a5b211..af727fccfe40 100644
> --- a/drivers/vhost/test.c
> +++ b/drivers/vhost/test.c
> @@ -308,6 +308,12 @@ static long vhost_test_set_backend(struct vhost_test *n, unsigned index, int fd)
> return r;
> }
>
> +static const int vhost_test_features[] = {
> + VHOST_FEATURES
> +};
> +
> +#define VHOST_TEST_FEATURES VHOST_FEATURES_U64(vhost_test_features, 0)
> +
> static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
> unsigned long arg)
> {
> @@ -328,14 +334,14 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
> return -EFAULT;
> return vhost_test_set_backend(n, backend.index, backend.fd);
> case VHOST_GET_FEATURES:
> - features = VHOST_FEATURES;
> + features = VHOST_TEST_FEATURES;
> if (copy_to_user(featurep, &features, sizeof features))
> return -EFAULT;
> return 0;
> case VHOST_SET_FEATURES:
> if (copy_from_user(&features, featurep, sizeof features))
> return -EFAULT;
> - if (features & ~VHOST_FEATURES)
> + if (features & ~VHOST_TEST_FEATURES)
> return -EOPNOTSUPP;
> return vhost_test_set_features(n, features);
> case VHOST_RESET_OWNER:
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 621a6d9a8791..d8f1af9a0ff1 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -14,6 +14,7 @@
> #include <linux/atomic.h>
> #include <linux/vhost_iotlb.h>
> #include <linux/irqbypass.h>
> +#include <linux/unroll.h>
>
> struct vhost_work;
> struct vhost_task;
> @@ -279,14 +280,39 @@ void vhost_iotlb_map_free(struct vhost_iotlb *iotlb,
> eventfd_signal((vq)->error_ctx);\
> } while (0)
>
> -enum {
> - VHOST_FEATURES = (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
> - (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
> - (1ULL << VIRTIO_RING_F_EVENT_IDX) |
> - (1ULL << VHOST_F_LOG_ALL) |
> - (1ULL << VIRTIO_F_ANY_LAYOUT) |
> - (1ULL << VIRTIO_F_VERSION_1)
> -};
> +#define VHOST_FEATURES \
> + VIRTIO_F_NOTIFY_ON_EMPTY, \
> + VIRTIO_RING_F_INDIRECT_DESC, \
> + VIRTIO_RING_F_EVENT_IDX, \
> + VHOST_F_LOG_ALL, \
> + VIRTIO_F_ANY_LAYOUT, \
> + VIRTIO_F_VERSION_1
> +
> +static inline u64 vhost_features_u64(const int *features, int size, int idx)
> +{
> + unsigned long res = 0;
Should this be u64?
> +
> + unrolled_count(VIRTIO_FEATURES_BITS)
> + for (int i = 0; i < size; ++i) {
> + int bit = features[i];
> +
> + if (virtio_features_chk_bit(bit) && VIRTIO_U64(bit) == idx)
> + res |= VIRTIO_BIT(bit);
> + }
> + return res;
> +}
> +
> +#define VHOST_FEATURES_U64(features, idx) \
> + vhost_features_u64(features, ARRAY_SIZE(features), idx)
> +
> +#define DEFINE_VHOST_FEATURES_ARRAY_ENTRY(idx, features) \
> + [idx] = VHOST_FEATURES_U64(features, idx),
> +
> +#define DEFINE_VHOST_FEATURES_ARRAY(array, features) \
> + u64 array[VIRTIO_FEATURES_U64S] = { \
> + UNROLL(VIRTIO_FEATURES_U64S, \
> + DEFINE_VHOST_FEATURES_ARRAY_ENTRY, features) \
> + }
>
> /**
> * vhost_vq_set_backend - Set backend.
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index ae01457ea2cd..16662f2b87c1 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -29,12 +29,14 @@
> */
> #define VHOST_VSOCK_PKT_WEIGHT 256
>
> -enum {
> - VHOST_VSOCK_FEATURES = VHOST_FEATURES |
> - (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
> - (1ULL << VIRTIO_VSOCK_F_SEQPACKET)
> +static const int vhost_vsock_features[] = {
> + VHOST_FEATURES,
> + VIRTIO_F_ACCESS_PLATFORM,
> + VIRTIO_VSOCK_F_SEQPACKET
> };
>
> +#define VHOST_VSOCK_FEATURES VHOST_FEATURES_U64(vhost_vsock_features, 0)
> +
> enum {
> VHOST_VSOCK_BACKEND_FEATURES = (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)
> };
> --
> MST
>
Thanks
Powered by blists - more mailing lists