On 7/21/25 9:49 AM, Akihiko Odaki wrote: > 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.
Indeed an optimal name here could be impossible to fit. IMHO VIRTIO_FEATURES_NU64S is the best option among the above and I like it. I'll use that one. Thanks, Paolo
