On 5/23/25 10:09 AM, Akihiko Odaki wrote: > On 2025/05/21 20:34, Paolo Abeni wrote: >> Use the extended types and helpers to manipulate the virtio_net >> features. >> >> Note that offloads are still 64bits wide, as per specification, >> and extended offloads will be mapped into such range. >> >> Signed-off-by: Paolo Abeni <pab...@redhat.com> >> --- >> hw/net/virtio-net.c | 87 +++++++++++++++++++++------------- >> include/hw/virtio/virtio-net.h | 2 +- >> 2 files changed, 55 insertions(+), 34 deletions(-) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 9f500c64e7..193469fc27 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -90,6 +90,17 @@ >> VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | >> \ >> VIRTIO_NET_RSS_HASH_TYPE_UDP_EX) >> >> +#define VIRTIO_OFFLOAD_MAP_MIN 46 >> +#define VIRTIO_OFFLOAD_MAP_LENGTH 4 >> +#define VIRTIO_OFFLOAD_MAP MAKE_64BIT_MASK(VIRTIO_OFFLOAD_MAP_MIN, \ >> + VIRTIO_OFFLOAD_MAP_LENGTH) >> +#define VIRTIO_FEATURES_MAP_MIN 65 >> +#define VIRTIO_O2F_DELTA (VIRTIO_FEATURES_MAP_MIN - \ >> + VIRTIO_OFFLOAD_MAP_MIN) >> + >> +#define VIRTIO_FEATURE_TO_OFFLOAD(fbit) (fbit >= 64 ? \ >> + fbit - VIRTIO_O2F_DELTA : fbit) >> + > > These are specific to virtio-net but look like they are common for > virtio as the names don't contain "NET". > > VIRTIO_FEATURES_MAP_MIN is also a bit confusing. It points to the least > significant bit that refers to an offloading feature in the upper-half > of the feature bits, but the name lacks the context.
Uhmmm... putting the whole context in the macro name sounds very verbose and/or hard, what about: How about VIRTIO_NET_OFFLOAD_MAPPED_MIN ? > @@ -862,13 +881,13 @@ static uint64_t > virtio_net_guest_offloads_by_features(uint64_t features) >> (1ULL << VIRTIO_NET_F_GUEST_USO4) | >> (1ULL << VIRTIO_NET_F_GUEST_USO6); >> >> - return guest_offloads_mask & features; >> + return guest_offloads_mask & virtio_net_features_to_offload(features); > > > How about: > > static const virtio_features_t guest_offload_features_mask = ... > virtio_features_t masked_features = guest_offload_features_mask & features; > > return masked_features | ((masked_features >> VIRTIO_FEATURES_MAP_MIN) > << VIRTIO_OFFLOAD_MAP_MIN); > > This makes virtio_net_features_to_offload() unnecessary. The above looks a little fragile, as (in future) 'features' could have some bit in the mapped range set (and not representing a guest offload): we need to explicitly mask such bits out before the first '&' operator. If dropping the helper is preferred, it can still be dropped. /P
