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
VIRTIO_DWORD -> VIRTIO_FEATURES_U64
(imho VIRTIO_U64 would sound ambiguous)
VIRTIO_BIT -> VIRTIO_FEATURES_BIT
(for consistency with the previous one)
>> +static inline bool virtio_features_equal(const uint64_t *f1,
>> + const uint64_t *f2)
>> +{
>> + return !memcmp(f1, f2, sizeof(uint64_t) * VIRTIO_FEATURES_DWORDS);
>> +}
>> +
>> +static inline bool virtio_features_use_extended(const uint64_t *features)
>
> "extended" should be abbreviated as "ex" for consistency.
Ok.
Thanks,
Paolo
