* [PATCH v4 0/2] virtio: feature related cleanups
@ 2025-11-16 7:45 Michael S. Tsirkin
2025-11-16 7:45 ` [PATCH v4 1/2] virtio: clean up features qword/dword terms Michael S. Tsirkin
2025-11-16 7:45 ` [PATCH v4 2/2] vhost: switch to arrays of feature bits Michael S. Tsirkin
0 siblings, 2 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-16 7:45 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Lunn, Paolo Abeni, Jason Wang, Eugenio Pérez,
Xuan Zhuo, Jonathan Corbet, kvm, virtualization, netdev,
linux-doc
Sorry about sending a bad v3, there was a chunk there
that does not belong at all.
Lightly tested.
Cleanup around handling of feature bits:
- address word/dword/qword confusion
- simplify interfaces so callers do not need to
remember in which 64 bit chunk each bit belongs
changes from v3:
drop an out of date unnecessary kdoc parser change
changes from v2:
- drop unnecessary casts
- rework the interface to use array of bits not
Michael S. Tsirkin (2):
virtio: clean up features qword/dword terms
vhost: switch to arrays of feature bits
drivers/vhost/net.c | 44 ++++++++++++++------------
drivers/vhost/scsi.c | 9 ++++--
drivers/vhost/test.c | 10 ++++--
drivers/vhost/vhost.h | 42 +++++++++++++++++++-----
drivers/vhost/vsock.c | 10 +++---
drivers/virtio/virtio.c | 12 +++----
drivers/virtio/virtio_debug.c | 10 +++---
drivers/virtio/virtio_pci_modern_dev.c | 6 ++--
include/linux/virtio.h | 2 +-
include/linux/virtio_config.h | 2 +-
include/linux/virtio_features.h | 29 +++++++++--------
include/linux/virtio_pci_modern.h | 8 ++---
12 files changed, 113 insertions(+), 71 deletions(-)
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/2] virtio: clean up features qword/dword terms
2025-11-16 7:45 [PATCH v4 0/2] virtio: feature related cleanups Michael S. Tsirkin
@ 2025-11-16 7:45 ` Michael S. Tsirkin
2025-11-19 1:54 ` Jason Wang
2025-11-16 7:45 ` [PATCH v4 2/2] vhost: switch to arrays of feature bits Michael S. Tsirkin
1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-16 7:45 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Lunn, Paolo Abeni, Jason Wang, Eugenio Pérez,
Xuan Zhuo, Jonathan Corbet, kvm, virtualization, netdev,
linux-doc
virtio pci uses word to mean "16 bits". mmio uses it to mean
"32 bits".
To avoid confusion, let's avoid the term in core virtio
altogether. Just say U64 to mean "64 bit".
Fixes: e7d4c1c5a546 ("virtio: introduce extended features")
Cc: "Paolo Abeni" <pabeni@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <492ef5aaa196d155d0535b5b6f4ad5b3fba70a1b.1761058528.git.mst@redhat.com>
---
drivers/vhost/net.c | 12 +++++------
drivers/virtio/virtio.c | 12 +++++------
drivers/virtio/virtio_debug.c | 10 ++++-----
drivers/virtio/virtio_pci_modern_dev.c | 6 +++---
include/linux/virtio.h | 2 +-
include/linux/virtio_config.h | 2 +-
include/linux/virtio_features.h | 29 +++++++++++++-------------
include/linux/virtio_pci_modern.h | 8 +++----
8 files changed, 41 insertions(+), 40 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 35ded4330431..d057ea55f5ad 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -69,7 +69,7 @@ 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_DWORDS] = {
+static const u64 vhost_net_features[VIRTIO_FEATURES_U64S] = {
VHOST_FEATURES |
(1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
(1ULL << VIRTIO_NET_F_MRG_RXBUF) |
@@ -1720,7 +1720,7 @@ static long vhost_net_set_owner(struct vhost_net *n)
static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
unsigned long arg)
{
- u64 all_features[VIRTIO_FEATURES_DWORDS];
+ u64 all_features[VIRTIO_FEATURES_U64S];
struct vhost_net *n = f->private_data;
void __user *argp = (void __user *)arg;
u64 __user *featurep = argp;
@@ -1752,7 +1752,7 @@ 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, VIRTIO_FEATURES_DWORDS);
+ copied = min(count, (u64)VIRTIO_FEATURES_U64S);
if (copy_to_user(argp, vhost_net_features,
copied * sizeof(u64)))
return -EFAULT;
@@ -1767,13 +1767,13 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
virtio_features_zero(all_features);
argp += sizeof(u64);
- copied = min(count, VIRTIO_FEATURES_DWORDS);
+ copied = min(count, (u64)VIRTIO_FEATURES_U64S);
if (copy_from_user(all_features, argp, copied * sizeof(u64)))
return -EFAULT;
/*
* Any feature specified by user-space above
- * VIRTIO_FEATURES_MAX is not supported by definition.
+ * VIRTIO_FEATURES_BITS is not supported by definition.
*/
for (i = copied; i < count; ++i) {
if (copy_from_user(&features, featurep + 1 + i,
@@ -1783,7 +1783,7 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
return -EOPNOTSUPP;
}
- for (i = 0; i < VIRTIO_FEATURES_DWORDS; i++)
+ for (i = 0; i < VIRTIO_FEATURES_U64S; i++)
if (all_features[i] & ~vhost_net_features[i])
return -EOPNOTSUPP;
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index a09eb4d62f82..5bdc6b82b30b 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -53,7 +53,7 @@ static ssize_t features_show(struct device *_d,
/* We actually represent this as a bitstring, as it could be
* arbitrary length in future. */
- for (i = 0; i < VIRTIO_FEATURES_MAX; i++)
+ for (i = 0; i < VIRTIO_FEATURES_BITS; i++)
len += sysfs_emit_at(buf, len, "%c",
__virtio_test_bit(dev, i) ? '1' : '0');
len += sysfs_emit_at(buf, len, "\n");
@@ -272,8 +272,8 @@ static int virtio_dev_probe(struct device *_d)
int err, i;
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
- u64 device_features[VIRTIO_FEATURES_DWORDS];
- u64 driver_features[VIRTIO_FEATURES_DWORDS];
+ u64 device_features[VIRTIO_FEATURES_U64S];
+ u64 driver_features[VIRTIO_FEATURES_U64S];
u64 driver_features_legacy;
/* We have a driver! */
@@ -286,7 +286,7 @@ static int virtio_dev_probe(struct device *_d)
virtio_features_zero(driver_features);
for (i = 0; i < drv->feature_table_size; i++) {
unsigned int f = drv->feature_table[i];
- if (!WARN_ON_ONCE(f >= VIRTIO_FEATURES_MAX))
+ if (!WARN_ON_ONCE(f >= VIRTIO_FEATURES_BITS))
virtio_features_set_bit(driver_features, f);
}
@@ -303,7 +303,7 @@ static int virtio_dev_probe(struct device *_d)
}
if (virtio_features_test_bit(device_features, VIRTIO_F_VERSION_1)) {
- for (i = 0; i < VIRTIO_FEATURES_DWORDS; ++i)
+ for (i = 0; i < VIRTIO_FEATURES_U64S; ++i)
dev->features_array[i] = driver_features[i] &
device_features[i];
} else {
@@ -325,7 +325,7 @@ static int virtio_dev_probe(struct device *_d)
goto err;
if (drv->validate) {
- u64 features[VIRTIO_FEATURES_DWORDS];
+ u64 features[VIRTIO_FEATURES_U64S];
virtio_features_copy(features, dev->features_array);
err = drv->validate(dev);
diff --git a/drivers/virtio/virtio_debug.c b/drivers/virtio/virtio_debug.c
index d58713ddf2e5..ccf1955a1183 100644
--- a/drivers/virtio/virtio_debug.c
+++ b/drivers/virtio/virtio_debug.c
@@ -8,12 +8,12 @@ static struct dentry *virtio_debugfs_dir;
static int virtio_debug_device_features_show(struct seq_file *s, void *data)
{
- u64 device_features[VIRTIO_FEATURES_DWORDS];
+ u64 device_features[VIRTIO_FEATURES_U64S];
struct virtio_device *dev = s->private;
unsigned int i;
virtio_get_features(dev, device_features);
- for (i = 0; i < VIRTIO_FEATURES_MAX; i++) {
+ for (i = 0; i < VIRTIO_FEATURES_BITS; i++) {
if (virtio_features_test_bit(device_features, i))
seq_printf(s, "%u\n", i);
}
@@ -26,7 +26,7 @@ static int virtio_debug_filter_features_show(struct seq_file *s, void *data)
struct virtio_device *dev = s->private;
unsigned int i;
- for (i = 0; i < VIRTIO_FEATURES_MAX; i++) {
+ for (i = 0; i < VIRTIO_FEATURES_BITS; i++) {
if (virtio_features_test_bit(dev->debugfs_filter_features, i))
seq_printf(s, "%u\n", i);
}
@@ -50,7 +50,7 @@ static int virtio_debug_filter_feature_add(void *data, u64 val)
{
struct virtio_device *dev = data;
- if (val >= VIRTIO_FEATURES_MAX)
+ if (val >= VIRTIO_FEATURES_BITS)
return -EINVAL;
virtio_features_set_bit(dev->debugfs_filter_features, val);
@@ -64,7 +64,7 @@ static int virtio_debug_filter_feature_del(void *data, u64 val)
{
struct virtio_device *dev = data;
- if (val >= VIRTIO_FEATURES_MAX)
+ if (val >= VIRTIO_FEATURES_BITS)
return -EINVAL;
virtio_features_clear_bit(dev->debugfs_filter_features, val);
diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index 9e503b7a58d8..413a8c353463 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -401,7 +401,7 @@ void vp_modern_get_extended_features(struct virtio_pci_modern_device *mdev,
int i;
virtio_features_zero(features);
- for (i = 0; i < VIRTIO_FEATURES_WORDS; i++) {
+ for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
u64 cur;
vp_iowrite32(i, &cfg->device_feature_select);
@@ -427,7 +427,7 @@ vp_modern_get_driver_extended_features(struct virtio_pci_modern_device *mdev,
int i;
virtio_features_zero(features);
- for (i = 0; i < VIRTIO_FEATURES_WORDS; i++) {
+ for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
u64 cur;
vp_iowrite32(i, &cfg->guest_feature_select);
@@ -448,7 +448,7 @@ void vp_modern_set_extended_features(struct virtio_pci_modern_device *mdev,
struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
int i;
- for (i = 0; i < VIRTIO_FEATURES_WORDS; i++) {
+ for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
u32 cur = features[i >> 1] >> (32 * (i & 1));
vp_iowrite32(i, &cfg->guest_feature_select);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 96c66126c074..132a474e5914 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -177,7 +177,7 @@ struct virtio_device {
union virtio_map vmap;
#ifdef CONFIG_VIRTIO_DEBUG
struct dentry *debugfs_dir;
- u64 debugfs_filter_features[VIRTIO_FEATURES_DWORDS];
+ u64 debugfs_filter_features[VIRTIO_FEATURES_U64S];
#endif
};
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index a1af2676bbe6..69f84ea85d71 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -80,7 +80,7 @@ struct virtqueue_info {
* Returns the first 64 feature bits.
* @get_extended_features:
* vdev: the virtio_device
- * Returns the first VIRTIO_FEATURES_MAX feature bits (all we currently
+ * Returns the first VIRTIO_FEATURES_BITS feature bits (all we currently
* need).
* @finalize_features: confirm what device features we'll be using.
* vdev: the virtio_device
diff --git a/include/linux/virtio_features.h b/include/linux/virtio_features.h
index f748f2f87de8..ea2ad8717882 100644
--- a/include/linux/virtio_features.h
+++ b/include/linux/virtio_features.h
@@ -4,15 +4,16 @@
#include <linux/bits.h>
-#define VIRTIO_FEATURES_DWORDS 2
-#define VIRTIO_FEATURES_MAX (VIRTIO_FEATURES_DWORDS * 64)
-#define VIRTIO_FEATURES_WORDS (VIRTIO_FEATURES_DWORDS * 2)
+#define VIRTIO_FEATURES_U64S 2
+#define VIRTIO_FEATURES_BITS (VIRTIO_FEATURES_U64S * 64)
+
#define VIRTIO_BIT(b) BIT_ULL((b) & 0x3f)
-#define VIRTIO_DWORD(b) ((b) >> 6)
+#define VIRTIO_U64(b) ((b) >> 6)
+
#define VIRTIO_DECLARE_FEATURES(name) \
union { \
u64 name; \
- u64 name##_array[VIRTIO_FEATURES_DWORDS];\
+ u64 name##_array[VIRTIO_FEATURES_U64S];\
}
static inline bool virtio_features_chk_bit(unsigned int bit)
@@ -22,9 +23,9 @@ static inline bool virtio_features_chk_bit(unsigned int bit)
* Don't care returning the correct value: the build
* will fail before any bad features access
*/
- BUILD_BUG_ON(bit >= VIRTIO_FEATURES_MAX);
+ BUILD_BUG_ON(bit >= VIRTIO_FEATURES_BITS);
} else {
- if (WARN_ON_ONCE(bit >= VIRTIO_FEATURES_MAX))
+ if (WARN_ON_ONCE(bit >= VIRTIO_FEATURES_BITS))
return false;
}
return true;
@@ -34,26 +35,26 @@ static inline bool virtio_features_test_bit(const u64 *features,
unsigned int bit)
{
return virtio_features_chk_bit(bit) &&
- !!(features[VIRTIO_DWORD(bit)] & VIRTIO_BIT(bit));
+ !!(features[VIRTIO_U64(bit)] & VIRTIO_BIT(bit));
}
static inline void virtio_features_set_bit(u64 *features,
unsigned int bit)
{
if (virtio_features_chk_bit(bit))
- features[VIRTIO_DWORD(bit)] |= VIRTIO_BIT(bit);
+ features[VIRTIO_U64(bit)] |= VIRTIO_BIT(bit);
}
static inline void virtio_features_clear_bit(u64 *features,
unsigned int bit)
{
if (virtio_features_chk_bit(bit))
- features[VIRTIO_DWORD(bit)] &= ~VIRTIO_BIT(bit);
+ features[VIRTIO_U64(bit)] &= ~VIRTIO_BIT(bit);
}
static inline void virtio_features_zero(u64 *features)
{
- memset(features, 0, sizeof(features[0]) * VIRTIO_FEATURES_DWORDS);
+ memset(features, 0, sizeof(features[0]) * VIRTIO_FEATURES_U64S);
}
static inline void virtio_features_from_u64(u64 *features, u64 from)
@@ -66,7 +67,7 @@ static inline bool virtio_features_equal(const u64 *f1, const u64 *f2)
{
int i;
- for (i = 0; i < VIRTIO_FEATURES_DWORDS; ++i)
+ for (i = 0; i < VIRTIO_FEATURES_U64S; ++i)
if (f1[i] != f2[i])
return false;
return true;
@@ -74,14 +75,14 @@ static inline bool virtio_features_equal(const u64 *f1, const u64 *f2)
static inline void virtio_features_copy(u64 *to, const u64 *from)
{
- memcpy(to, from, sizeof(to[0]) * VIRTIO_FEATURES_DWORDS);
+ memcpy(to, from, sizeof(to[0]) * VIRTIO_FEATURES_U64S);
}
static inline void virtio_features_andnot(u64 *to, const u64 *f1, const u64 *f2)
{
int i;
- for (i = 0; i < VIRTIO_FEATURES_DWORDS; i++)
+ for (i = 0; i < VIRTIO_FEATURES_U64S; i++)
to[i] = f1[i] & ~f2[i];
}
diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
index 48bc12d1045b..9a3f2fc53bd6 100644
--- a/include/linux/virtio_pci_modern.h
+++ b/include/linux/virtio_pci_modern.h
@@ -107,7 +107,7 @@ void vp_modern_set_extended_features(struct virtio_pci_modern_device *mdev,
static inline u64
vp_modern_get_features(struct virtio_pci_modern_device *mdev)
{
- u64 features_array[VIRTIO_FEATURES_DWORDS];
+ u64 features_array[VIRTIO_FEATURES_U64S];
vp_modern_get_extended_features(mdev, features_array);
return features_array[0];
@@ -116,11 +116,11 @@ vp_modern_get_features(struct virtio_pci_modern_device *mdev)
static inline u64
vp_modern_get_driver_features(struct virtio_pci_modern_device *mdev)
{
- u64 features_array[VIRTIO_FEATURES_DWORDS];
+ u64 features_array[VIRTIO_FEATURES_U64S];
int i;
vp_modern_get_driver_extended_features(mdev, features_array);
- for (i = 1; i < VIRTIO_FEATURES_DWORDS; ++i)
+ for (i = 1; i < VIRTIO_FEATURES_U64S; ++i)
WARN_ON_ONCE(features_array[i]);
return features_array[0];
}
@@ -128,7 +128,7 @@ vp_modern_get_driver_features(struct virtio_pci_modern_device *mdev)
static inline void
vp_modern_set_features(struct virtio_pci_modern_device *mdev, u64 features)
{
- u64 features_array[VIRTIO_FEATURES_DWORDS];
+ u64 features_array[VIRTIO_FEATURES_U64S];
virtio_features_from_u64(features_array, features);
vp_modern_set_extended_features(mdev, features_array);
--
MST
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 2/2] vhost: switch to arrays of feature bits
2025-11-16 7:45 [PATCH v4 0/2] virtio: feature related cleanups Michael S. Tsirkin
2025-11-16 7:45 ` [PATCH v4 1/2] virtio: clean up features qword/dword terms Michael S. Tsirkin
@ 2025-11-16 7:45 ` Michael S. Tsirkin
2025-11-19 2:03 ` Jason Wang
1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-16 7:45 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Lunn, Paolo Abeni, Jason Wang, Eugenio Pérez,
Xuan Zhuo, Jonathan Corbet, kvm, virtualization, netdev,
linux-doc, Mike Christie, Paolo Bonzini, Stefan Hajnoczi,
Stefano Garzarella
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@redhat.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;
+ }
/* 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;
+
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] virtio: clean up features qword/dword terms
2025-11-16 7:45 ` [PATCH v4 1/2] virtio: clean up features qword/dword terms Michael S. Tsirkin
@ 2025-11-19 1:54 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2025-11-19 1:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Andrew Lunn, Paolo Abeni, Eugenio Pérez,
Xuan Zhuo, Jonathan Corbet, kvm, virtualization, netdev,
linux-doc
On Sun, Nov 16, 2025 at 3:45 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> virtio pci uses word to mean "16 bits". mmio uses it to mean
> "32 bits".
>
> To avoid confusion, let's avoid the term in core virtio
> altogether. Just say U64 to mean "64 bit".
>
> Fixes: e7d4c1c5a546 ("virtio: introduce extended features")
> Cc: "Paolo Abeni" <pabeni@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Message-Id: <492ef5aaa196d155d0535b5b6f4ad5b3fba70a1b.1761058528.git.mst@redhat.com>
> ---
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] vhost: switch to arrays of feature bits
2025-11-16 7:45 ` [PATCH v4 2/2] vhost: switch to arrays of feature bits Michael S. Tsirkin
@ 2025-11-19 2:03 ` Jason Wang
2025-11-19 6:26 ` Michael S. Tsirkin
0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2025-11-19 2:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Andrew Lunn, Paolo Abeni, Eugenio Pérez,
Xuan Zhuo, Jonathan Corbet, kvm, virtualization, netdev,
linux-doc, Mike Christie, Paolo Bonzini, Stefan Hajnoczi,
Stefano Garzarella
On Sun, Nov 16, 2025 at 3:45 PM Michael S. Tsirkin <mst@redhat.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@redhat.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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] vhost: switch to arrays of feature bits
2025-11-19 2:03 ` Jason Wang
@ 2025-11-19 6:26 ` Michael S. Tsirkin
0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-19 6:26 UTC (permalink / raw)
To: Jason Wang
Cc: linux-kernel, Andrew Lunn, Paolo Abeni, Eugenio Pérez,
Xuan Zhuo, Jonathan Corbet, kvm, virtualization, netdev,
linux-doc, Mike Christie, Paolo Bonzini, Stefan Hajnoczi,
Stefano Garzarella
On Wed, Nov 19, 2025 at 10:03:58AM +0800, Jason Wang wrote:
> On Sun, Nov 16, 2025 at 3:45 PM Michael S. Tsirkin <mst@redhat.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@redhat.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?
Just so we can name the variable "features", as well as
having the declaration stand out a bit more.
> >
> > /* 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?
Ugh. Yes.
> > +
> > + 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-19 6:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-16 7:45 [PATCH v4 0/2] virtio: feature related cleanups Michael S. Tsirkin
2025-11-16 7:45 ` [PATCH v4 1/2] virtio: clean up features qword/dword terms Michael S. Tsirkin
2025-11-19 1:54 ` Jason Wang
2025-11-16 7:45 ` [PATCH v4 2/2] vhost: switch to arrays of feature bits Michael S. Tsirkin
2025-11-19 2:03 ` Jason Wang
2025-11-19 6:26 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).