On 2025/07/21 16:33, Paolo Abeni wrote:
On 7/20/25 12:41 PM, Akihiko Odaki wrote:
On 2025/07/18 17:52, Paolo Abeni wrote:
diff --git a/include/hw/virtio/virtio-features.h
b/include/hw/virtio/virtio-features.h
new file mode 100644
index 0000000000..68e326e3e8
--- /dev/null
+++ b/include/hw/virtio/virtio-features.h
@@ -0,0 +1,123 @@
+/*
+ * 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_BIT(b) BIT_ULL((b) % 64)
+#define VIRTIO_DWORD(b) ((b) >> 6)
+#define VIRTIO_FEATURES_WORDS (VIRTIO_FEATURES_MAX >> 5)
+#define VIRTIO_FEATURES_DWORDS (VIRTIO_FEATURES_WORDS >> 1)
These shifts are better to be replaced with division for clarity;
BIT_WORD() is a good example.
"WORD" and "DWORD" should be avoided due to contradicting definitions
common in QEMU as described at:
https://lore.kernel.org/qemu-devel/aab8c434-364e-4305-9d8b-943eb0c98...@rsg.ci.i.u-tokyo.ac.jp/
BITS_TO_U32S() is a good example this regard.
Ok, I'll rename:
VIRTIO_FEATURES_DWORDS -> VIRTIO_U64_PER_FEATURES
VIRTIO_FEATURES_WORDS -> VIRTIO_U32_PER_FEATURES
U64 and U32 should be plural (i.e., rename them into
VIRTIO_U64S_PER_FEATURES and VIRTIO_U32S_PER_FEATURES)
"PER_FEATURES" also sounds a bit awkward; BITS_PER_BYTE and
BITS_PER_LONG had singular after "per" so the unit was clear, but it is
not in this case.
I could think of several options:
- VIRTIO_U64S_PER_FEATURES (what you proposed + plural U64S)
- VIRTIO_FEATURES_U64S (closer to the previous version)
- VIRTIO_FEATURES_NU64S (like CPU_TEMP_BUF_NLONGS)
- VIRTIO_U64S_PER_FEATURE_BITMASK
They have downsides and upsides, and I don't have an idea what's the best.
Regards,
Akihiko Odaki