On Wed, 17 Sept 2025 at 14:12, Stefano Garzarella <[email protected]> wrote:
>
> On Fri, Sep 12, 2025 at 03:06:55PM +0200, Paolo Abeni wrote:
> >The virtio specifications allows for up to 128 bits for the
> >device features. Soon we are going to use some of the 'extended'
> >bits features (bit 64 and above) for the virtio net driver.
> >
> >Represent the virtio features bitmask with a fixes size array, and

s/fixes/fixed

But again, my A-b remains, we can fix only if there will be a v7.

Stefano

> >introduce a few helpers to help manipulate them.
> >
> >Most drivers will keep using only 64 bits features space: use union
> >to allow them access the lower part of the extended space without any
> >per driver change.
> >
> >Reviewed-by: Akihiko Odaki <[email protected]>
> >Acked-by: Jason Wang <[email protected]>
> >Signed-off-by: Paolo Abeni <[email protected]>
> >---
> >v5 -> v6:
> >  - removed trailing EoL
> >
> >v4 -> v5:
> >  - DEFINE_PROP_FEATURE -> VIRTIO_DEFINE_PROP_FEATURE
> >
> >v3 -> v4:
> >  - VIRTIO_FEATURES_DWORDS -> VIRTIO_FEATURES_NU64S
> >  - VIRTIO_FEATURES_WORDS -> VIRTIO_FEATURES_NU32S
> >  - VIRTIO_DWORD ->  VIRTIO_FEATURES_U64
> >  - VIRTIO_BIT -> VIRTIO_FEATURES_BIT
> >  - virtio_features_use_extended -> virtio_features_use_ex
> >  - move DEFINE_PROP_FEATURE definition here
> >
> >v2 -> v3:
> >  - fix preprocessor guard name
> >  - use BIT_ULL
> >  - add missing parentheses
> >  - use memcmp()
> >  - _is_empty() -> _empty()
> >  - _andnot() returns a bool, true if dst has any non zero bits
> >  - _array -> _ex
> >
> >v1 -> v2:
> >  - use a fixed size array for features instead of uint128
> >  - use union with u64 to reduce the needed code churn
> >---
> > include/hw/virtio/virtio-features.h | 126 ++++++++++++++++++++++++++++
> > include/hw/virtio/virtio.h          |   7 +-
> > 2 files changed, 130 insertions(+), 3 deletions(-)
> > create mode 100644 include/hw/virtio/virtio-features.h
>
> Acked-by: Stefano Garzarella <[email protected]>
>
> >
> >diff --git a/include/hw/virtio/virtio-features.h 
> >b/include/hw/virtio/virtio-features.h
> >new file mode 100644
> >index 0000000000..e29b7fe48f
> >--- /dev/null
> >+++ b/include/hw/virtio/virtio-features.h
> >@@ -0,0 +1,126 @@
> >+/*
> >+ * Virtio features helpers
> >+ *
> >+ * Copyright 2025 Red Hat, Inc.
> >+ *
> >+ * SPDX-License-Identifier: GPL-2.0-or-later
> >+ */
> >+
> >+#ifndef QEMU_VIRTIO_FEATURES_H
> >+#define QEMU_VIRTIO_FEATURES_H
> >+
> >+#include "qemu/bitops.h"
> >+
> >+#define VIRTIO_FEATURES_FMT        "%016"PRIx64"%016"PRIx64
> >+#define VIRTIO_FEATURES_PR(f)      (f)[1], (f)[0]
> >+
> >+#define VIRTIO_FEATURES_MAX        128
> >+#define VIRTIO_FEATURES_BIT(b)     BIT_ULL((b) % 64)
> >+#define VIRTIO_FEATURES_U64(b)     ((b) / 64)
> >+#define VIRTIO_FEATURES_NU32S      (VIRTIO_FEATURES_MAX / 32)
> >+#define VIRTIO_FEATURES_NU64S      (VIRTIO_FEATURES_MAX / 64)
> >+
> >+#define VIRTIO_DECLARE_FEATURES(name)                        \
> >+    union {                                                  \
> >+        uint64_t name;                                       \
> >+        uint64_t name##_ex[VIRTIO_FEATURES_NU64S];           \
> >+    }
> >+
> >+#define VIRTIO_DEFINE_PROP_FEATURE(_name, _state, _field, _bit, _defval)   \
> >+    DEFINE_PROP_BIT64(_name, _state, _field[VIRTIO_FEATURES_U64(_bit)],    \
> >+                      (_bit) % 64, _defval)
> >+
> >+static inline void virtio_features_clear(uint64_t *features)
> >+{
> >+    memset(features, 0, sizeof(features[0]) * VIRTIO_FEATURES_NU64S);
> >+}
> >+
> >+static inline void virtio_features_from_u64(uint64_t *features, uint64_t 
> >from)
> >+{
> >+    virtio_features_clear(features);
> >+    features[0] = from;
> >+}
> >+
> >+static inline bool virtio_has_feature_ex(const uint64_t *features,
> >+                                         unsigned int fbit)
> >+{
> >+    assert(fbit < VIRTIO_FEATURES_MAX);
> >+    return features[VIRTIO_FEATURES_U64(fbit)] & VIRTIO_FEATURES_BIT(fbit);
> >+}
> >+
> >+static inline void virtio_add_feature_ex(uint64_t *features,
> >+                                         unsigned int fbit)
> >+{
> >+    assert(fbit < VIRTIO_FEATURES_MAX);
> >+    features[VIRTIO_FEATURES_U64(fbit)] |= VIRTIO_FEATURES_BIT(fbit);
> >+}
> >+
> >+static inline void virtio_clear_feature_ex(uint64_t *features,
> >+                                           unsigned int fbit)
> >+{
> >+    assert(fbit < VIRTIO_FEATURES_MAX);
> >+    features[VIRTIO_FEATURES_U64(fbit)] &= ~VIRTIO_FEATURES_BIT(fbit);
> >+}
> >+
> >+static inline bool virtio_features_equal(const uint64_t *f1,
> >+                                         const uint64_t *f2)
> >+{
> >+    return !memcmp(f1, f2, sizeof(uint64_t) * VIRTIO_FEATURES_NU64S);
> >+}
> >+
> >+static inline bool virtio_features_use_ex(const uint64_t *features)
> >+{
> >+    int i;
> >+
> >+    for (i = 1; i < VIRTIO_FEATURES_NU64S; ++i) {
> >+        if (features[i]) {
> >+            return true;
> >+        }
> >+    }
> >+    return false;
> >+}
> >+
> >+static inline bool virtio_features_empty(const uint64_t *features)
> >+{
> >+    return !virtio_features_use_ex(features) && !features[0];
> >+}
> >+
> >+static inline void virtio_features_copy(uint64_t *to, const uint64_t *from)
> >+{
> >+    memcpy(to, from, sizeof(to[0]) * VIRTIO_FEATURES_NU64S);
> >+}
> >+
> >+static inline bool virtio_features_andnot(uint64_t *to, const uint64_t *f1,
> >+                                           const uint64_t *f2)
> >+{
> >+    uint64_t diff = 0;
> >+    int i;
> >+
> >+    for (i = 0; i < VIRTIO_FEATURES_NU64S; i++) {
> >+        to[i] = f1[i] & ~f2[i];
> >+        diff |= to[i];
> >+    }
> >+    return diff;
> >+}
> >+
> >+static inline void virtio_features_and(uint64_t *to, const uint64_t *f1,
> >+                                       const uint64_t *f2)
> >+{
> >+    int i;
> >+
> >+    for (i = 0; i < VIRTIO_FEATURES_NU64S; i++) {
> >+        to[i] = f1[i] & f2[i];
> >+    }
> >+}
> >+
> >+static inline void virtio_features_or(uint64_t *to, const uint64_t *f1,
> >+                                       const uint64_t *f2)
> >+{
> >+    int i;
> >+
> >+    for (i = 0; i < VIRTIO_FEATURES_NU64S; i++) {
> >+        to[i] = f1[i] | f2[i];
> >+    }
> >+}
> >+
> >+#endif
> >diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >index c594764f23..39e4059a66 100644
> >--- a/include/hw/virtio/virtio.h
> >+++ b/include/hw/virtio/virtio.h
> >@@ -16,6 +16,7 @@
> >
> > #include "system/memory.h"
> > #include "hw/qdev-core.h"
> >+#include "hw/virtio/virtio-features.h"
> > #include "net/net.h"
> > #include "migration/vmstate.h"
> > #include "qemu/event_notifier.h"
> >@@ -121,9 +122,9 @@ struct VirtIODevice
> >      * backend (e.g. vhost) and could potentially be a subset of the
> >      * total feature set offered by QEMU.
> >      */
> >-    uint64_t host_features;
> >-    uint64_t guest_features;
> >-    uint64_t backend_features;
> >+    VIRTIO_DECLARE_FEATURES(host_features);
> >+    VIRTIO_DECLARE_FEATURES(guest_features);
> >+    VIRTIO_DECLARE_FEATURES(backend_features);
> >
> >     size_t config_len;
> >     void *config;
> >--
> >2.51.0
> >


Reply via email to