virtio: introduce support for GSO over UDP tunnel
Paolo Abeni
maintainerv4 cover virtio: introduce support for GSO over UDP tunnel
Some virtualized deployments use UDP tunnel pervasively and are impacted negatively by the lack of GSO support for such kind of traffic in the virtual NIC driver.
The virtio_net specification recently introduced support for GSO over UDP tunnel, and the kernel side of the implementation has been merged into the net-next tree; this series updates the virtio implementation to support such a feature.
Currently the qemu virtio support limits the feature space to 64 bits, while the virtio specification allows for a larger number of features. Specifically the GSO-over-UDP-tunnel-related virtio features use bits 65-69; the larger part of this series (patches 4-12) actually deals with extending the features space.
The extended features are carried by fixed size uint64_t arrays, bringing the current maximum features number to 128.
The patches use some syntactic sugar to try to minimize the otherwise very large code churn. Specifically the extended features are boundled in an union with 'legacy' features definition, allowing no changes in the virtio devices not needing the extended features set.
The actual offload implementation is in patches 13 and 14 and boils down to propagating the new offload to the tun devices and the vhost backend.
Patch 1 is a small pre-req refactor that ideally could enter the tree separately; it's presented here in the same series to help reviewers more easily getting the full picture, patch 2 updates to linux headers update script to deal with annotations recently introduce in the kernel and patch 3 is a needed linux headers update.
Tested with basic stream transfer with all the possible permutations of host kernel/qemu/guest kernel with/without GSO over UDP tunnel support, vs snapshots creation and restore and vs migration.
Sharing again as RFC as the kernel bits have not entered the Linus tree yet - but they should on next merge window.
---
Note: next revision will be after the Linux merge window to allow dropping the 'RFC' tag and also due to myself being off-the-grid meanwhile.
v3 -> v4:
- more sparse cleanups
- fix build failures on a couple of targets
v2 -> v3:
- consolidated suffixes for new fields to '_ex'
- avoid pre/post load trickery and relay on reset zeroing the features
- cleaned-up virtio store implementation deduplicating a bit of code
- many more cleanups, see the individual patches changelog for the details
- I left patch 1 unmodified, still some hope we could live with that;)
Paolo Abeni (14):
net: bundle all offloads in a single struct
linux-headers: deal with counted_by annotation
linux-headers: Update to Linux ~v6.16-rc7 net-next
virtio: introduce extended features type
virtio: serialize extended features state
virtio: add support for negotiating extended features
virtio-pci: implement support for extended features
vhost: add support for negotiating extended features
qmp: update virtio features map to support extended features
vhost-backend: implement extended features support
vhost-net: implement extended features support
virtio-net: implement extended features support
net: implement tunnel probing
net: implement UDP tunnel features offloading
hw/net/e1000e_core.c | 5 +- hw/net/igb_core.c | 5 +- hw/net/vhost_net-stub.c | 8 +- hw/net/vhost_net.c | 45 ++-- hw/net/virtio-net.c | 222 +++++++++++++------ hw/net/vmxnet3.c | 13 +- hw/virtio/vhost-backend.c | 62 +++++- hw/virtio/vhost.c | 68 ++++-- hw/virtio/virtio-bus.c | 11 +- hw/virtio/virtio-hmp-cmds.c | 3 +- hw/virtio/virtio-pci.c | 69 +++++- hw/virtio/virtio-qmp.c | 91 +++++--- hw/virtio/virtio-qmp.h | 3 +- hw/virtio/virtio.c | 102 ++++++--- include/hw/virtio/vhost-backend.h | 6 + include/hw/virtio/vhost.h | 56 ++++- include/hw/virtio/virtio-features.h | 127 +++++++++++ include/hw/virtio/virtio-net.h | 2 +- include/hw/virtio/virtio-pci.h | 2 +- include/hw/virtio/virtio.h | 11 +- include/net/net.h | 20 +- include/net/vhost_net.h | 33 ++- include/standard-headers/linux/ethtool.h | 4 +- include/standard-headers/linux/vhost_types.h | 5 + include/standard-headers/linux/virtio_net.h | 33 +++ linux-headers/linux/vhost.h | 7 + net/net.c | 17 +- net/netmap.c | 3 +- net/tap-bsd.c | 8 +- net/tap-linux.c | 38 +++- net/tap-linux.h | 9 + net/tap-solaris.c | 9 +- net/tap-stub.c | 8 +- net/tap.c | 21 +- net/tap_int.h | 5 +- qapi/virtio.json | 9 +- scripts/update-linux-headers.sh | 1 + 37 files changed, 875 insertions(+), 266 deletions(-) create mode 100644 include/hw/virtio/virtio-features.h
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 1/14 net: bundle all offloads in a single struct
The set_offload() argument list is already pretty long and we are going to introduce soon a bunch of additional offloads.
Replace the offload arguments with a single struct and update all the relevant call-sites.
No functional changes intended.
---
v3 -> v4:
- style fix for NetOffloads init
v1 -> v2:
- drop unneeded 'struct' keywords
- moved to series start
--- hw/net/e1000e_core.c | 5 +++-- hw/net/igb_core.c | 5 +++-- hw/net/virtio-net.c | 19 +++++++++++-------- hw/net/vmxnet3.c | 13 +++++-------- include/net/net.h | 15 ++++++++++++--- net/net.c | 5 ++--- net/netmap.c | 3 +-- net/tap-bsd.c | 3 +-- net/tap-linux.c | 21 ++++++++++++--------- net/tap-solaris.c | 4 ++-- net/tap-stub.c | 3 +-- net/tap.c | 8 ++++---- net/tap_int.h | 4 ++-- 13 files changed, 59 insertions(+), 49 deletions(-)
hw/net/e1000e_core.c +3 -2
@@ -2827,8 +2827,9 @@ e1000e_update_rx_offloads(E1000ECore *core)trace_e1000e_rx_set_cso (cso_state);if (core->has_vnet) { -qemu_set_offload (qemu_get_queue (core->owner_nic)->peer, - cso_state,0 ,0 ,0 ,0 ,0 ,0 ); +NetOffloads ol = { .csum = cso_state }; + +qemu_set_offload (qemu_get_queue (core->owner_nic)->peer, &ol); } }
hw/net/igb_core.c +3 -2
@@ -3058,8 +3058,9 @@ igb_update_rx_offloads(IGBCore *core)trace_e1000e_rx_set_cso (cso_state);if (core->has_vnet) { -qemu_set_offload (qemu_get_queue (core->owner_nic)->peer, - cso_state,0 ,0 ,0 ,0 ,0 ,0 ); +NetOffloads ol = {.csum = cso_state }; + +qemu_set_offload (qemu_get_queue (core->owner_nic)->peer, &ol); } }
hw/net/virtio-net.c +11 -8
@@ -773,14 +773,17 @@ static uint64_t virtio_net_bad_features(VirtIODevice *vdev)static void virtio_net_apply_guest_offloads (VirtIONet *n) { -qemu_set_offload (qemu_get_queue (n->nic)->peer, - !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_CSUM )), - !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_TSO4 )), - !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_TSO6 )), - !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_ECN )), - !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_UFO )), - !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_USO4 )), - !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_USO6 ))); +NetOffloads ol = { + .csum = !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_CSUM )), + .tso4 = !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_TSO4 )), + .tso6 = !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_TSO6 )), + .ecn = !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_ECN )), + .ufo = !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_UFO )), + .uso4 = !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_USO4 )), + .uso6 = !!(n->curr_guest_offloads & (1ULL <<VIRTIO_NET_F_GUEST_USO6 )), + }; + +qemu_set_offload (qemu_get_queue (n->nic)->peer, &ol); }static uint64_t virtio_net_guest_offloads_by_features (uint64_t features)
hw/net/vmxnet3.c +5 -8
@@ -1322,14 +1322,11 @@ static void vmxnet3_update_features(VMXNET3State *s) s->lro_supported, rxcso_supported, s->rx_vlan_stripping);if (s->peer_has_vhdr) { -qemu_set_offload (qemu_get_queue (s->nic)->peer, - rxcso_supported, - s->lro_supported, - s->lro_supported, -0 , -0 , -0 , -0 ); +NetOffloads ol = { .csum = rxcso_supported, + .tso4 = s->lro_supported, + .tso6 = s->lro_supported }; + +qemu_set_offload (qemu_get_queue (s->nic)->peer, &ol); } }
include/net/net.h +12 -3
@@ -35,6 +35,16 @@ typedef struct NICConf {int32_t bootindex; } NICConf; +typedef struct NetOffloads { +bool csum; +bool tso4; +bool tso6; +bool ecn; +bool ufo; +bool uso4; +bool uso6; +}NetOffloads ; +#define DEFINE_NIC_PROPERTIES (_state, _conf) \ DEFINE_PROP_MACADDR("mac", _state, _conf.macaddr), \ DEFINE_PROP_NETDEV("netdev", _state, _conf.peers)
@@ -57,7 +67,7 @@ typedef bool (HasUfo)(NetClientState *);typedef bool (HasUso )(NetClientState *);typedef bool (HasVnetHdr )(NetClientState *);typedef bool (HasVnetHdrLen )(NetClientState *,int ); -typedef void (SetOffload )(NetClientState *,int ,int ,int ,int ,int ,int ,int ); +typedef void (SetOffload )(NetClientState *,const NetOffloads *);typedef int (GetVnetHdrLen )(NetClientState *);typedef void (SetVnetHdrLen )(NetClientState *,int );typedef bool (GetVnetHashSupportedTypes )(NetClientState *,uint32_t *);
@@ -189,8 +199,7 @@ bool qemu_has_ufo(NetClientState *nc);bool qemu_has_uso (NetClientState *nc);bool qemu_has_vnet_hdr (NetClientState *nc);bool qemu_has_vnet_hdr_len (NetClientState *nc,int len); -void qemu_set_offload (NetClientState *nc,int csum,int tso4,int tso6, -int ecn,int ufo,int uso4,int uso6); +void qemu_set_offload (NetClientState *nc,const NetOffloads *ol);int qemu_get_vnet_hdr_len (NetClientState *nc);void qemu_set_vnet_hdr_len (NetClientState *nc,int len);bool qemu_get_vnet_hash_supported_types (NetClientState *nc,uint32_t *types);
net/net.c +2 -3
@@ -540,14 +540,13 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len)return nc->info->has_vnet_hdr_len (nc, len); } -void qemu_set_offload (NetClientState *nc,int csum,int tso4,int tso6, -int ecn,int ufo,int uso4,int uso6) +void qemu_set_offload (NetClientState *nc,const NetOffloads *ol) {if (!nc || !nc->info->set_offload) {return ; } - nc->info->set_offload (nc, csum, tso4, tso6, ecn, ufo, uso4, uso6); + nc->info->set_offload (nc, ol); }int qemu_get_vnet_hdr_len (NetClientState *nc)
net/netmap.c +1 -2
@@ -366,8 +366,7 @@ static void netmap_set_vnet_hdr_len(NetClientState *nc, int len) } } -static void netmap_set_offload (NetClientState *nc,int csum,int tso4,int tso6, -int ecn,int ufo,int uso4,int uso6) +static void netmap_set_offload (NetClientState *nc,const NetOffloads *ol) {NetmapState *s =DO_UPCAST (NetmapState, nc, nc);
net/tap-bsd.c +1 -2
@@ -231,8 +231,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)return -EINVAL ; } -void tap_fd_set_offload (int fd,int csum,int tso4, -int tso6,int ecn,int ufo,int uso4,int uso6) +void tap_fd_set_offload (int fd,const NetOffloads *ol) { }
net/tap-linux.c +12 -9
@@ -239,8 +239,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)abort (); } -void tap_fd_set_offload (int fd,int csum,int tso4, -int tso6,int ecn,int ufo,int uso4,int uso6) +void tap_fd_set_offload (int fd,const NetOffloads *ol) {unsigned int offload =0 ;
@@ -249,20 +248,24 @@ void tap_fd_set_offload(int fd, int csum, int tso4,return ; } -if (csum) { +if (ol->csum) { offload |=TUN_F_CSUM ; -if (tso4) +if (ol->tso4) { offload |=TUN_F_TSO4 ; -if (tso6) + } +if (ol->tso6) { offload |=TUN_F_TSO6 ; -if ((tso4 || tso6) && ecn) + } +if ((ol->tso4 || ol->tso6) && ol->ecn) { offload |=TUN_F_TSO_ECN ; -if (ufo) + } +if (ol->ufo) { offload |=TUN_F_UFO ; -if (uso4) { + } +if (ol->uso4) { offload |=TUN_F_USO4 ; } -if (uso6) { +if (ol->uso6) { offload |=TUN_F_USO6 ; } }
net/tap-solaris.c +2 -2
@@ -27,6 +27,7 @@#include "tap_int.h" #include "qemu/ctype.h" #include "qemu/cutils.h" +#include "net/net.h" #include <sys/ethernet.h> #include <sys/sockio.h>
@@ -235,8 +236,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)return -EINVAL ; } -void tap_fd_set_offload (int fd,int csum,int tso4, -int tso6,int ecn,int ufo,int uso4,int uso6) +void tap_fd_set_offload (int fd,const NetOffloads *ol) { }
net/tap-stub.c +1 -2
@@ -66,8 +66,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)return -EINVAL ; } -void tap_fd_set_offload (int fd,int csum,int tso4, -int tso6,int ecn,int ufo,int uso4,int uso6) +void tap_fd_set_offload (int fd,const NetOffloads *ol) { }
net/tap.c +4 -4
@@ -280,15 +280,14 @@ static int tap_set_vnet_be(NetClientState *nc, bool is_be)return tap_fd_set_vnet_be (s->fd, is_be); } -static void tap_set_offload (NetClientState *nc,int csum,int tso4, -int tso6,int ecn,int ufo,int uso4,int uso6) +static void tap_set_offload (NetClientState *nc,const NetOffloads *ol) {TAPState *s =DO_UPCAST (TAPState, nc, nc);if (s->fd <0 ) {return ; } -tap_fd_set_offload (s->fd, csum, tso4, tso6, ecn, ufo, uso4, uso6); +tap_fd_set_offload (s->fd, ol); }static void tap_exit_notify (Notifier *notifier,void *data)
@@ -386,6 +385,7 @@ static TAPState *net_tap_fd_init(NetClientState *peer, int fd, int vnet_hdr) { +NetOffloads ol = {};NetClientState *nc;TAPState *s;
@@ -399,7 +399,7 @@ static TAPState *net_tap_fd_init(NetClientState *peer, s->has_ufo =tap_probe_has_ufo (s->fd); s->has_uso =tap_probe_has_uso (s->fd); s->enabled = true; -tap_set_offload (&s->nc,0 ,0 ,0 ,0 ,0 ,0 ,0 ); +tap_set_offload (&s->nc, &ol);/* * Make sure host header length is set correctly in tap: * it might have been modified by another instance of qemu.
net/tap_int.h +2 -2
@@ -27,6 +27,7 @@#define NET_TAP_INT_H #include "qapi/qapi-types-net.h" +#include "net/net.h" int tap_open (char *ifname,int ifname_size,int *vnet_hdr,int vnet_hdr_required,int mq_required,Error **errp);
@@ -37,8 +38,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);int tap_probe_vnet_hdr (int fd,Error **errp);int tap_probe_has_ufo (int fd);int tap_probe_has_uso (int fd); -void tap_fd_set_offload (int fd,int csum,int tso4,int tso6,int ecn,int ufo, -int uso4,int uso6); +void tap_fd_set_offload (int fd,const NetOffloads *ol);void tap_fd_set_vnet_hdr_len (int fd,int len);int tap_fd_set_vnet_le (int fd,int vnet_is_le);int tap_fd_set_vnet_be (int fd,int vnet_is_be);
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 4/14 virtio: introduce extended features type
In reply to Paolo AbeniThe 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 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.
---
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 | 127 ++++++++++++++++++++++++++++ include/hw/virtio/virtio.h | 7 +- 2 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 include/hw/virtio/virtio-features.h
include/hw/virtio/virtio-features.h new file +127 -0
@@ -0,0 +1,127 @@ +/* +* 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 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 +
include/hw/virtio/virtio.h +4 -3
@@ -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;
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 5/14 virtio: serialize extended features state
In reply to Paolo AbeniIf the driver uses any of the extended features (i.e. 64 or above), store the extended features range (64-127 bits).
At load time, let legacy features initialize the full features range and pass it to the set helper; sub-states loading will have filled-up the extended part as needed.
This is one of the few spots that need explicitly to know and set in stone the extended features array size; add a build bug to prevent breaking the migration should such size change again in the future:
more serialization plumbing will be needed.
---
v3 -> v4:
- move the build bug before vmstate_virtio
- use new virtio_features macro names
v2 -> v3:
- 128bit_features state load/stores only the high bits
- consolidate the load implementation to use a single set helper for 128/64/32 bits features
- _array -> _ex
v1 -> v2:
- uint128_t -> u64[2]
--- hw/virtio/virtio.c | 88 ++++++++++++++++++++++++++++++---------------- 1 file changed, 57 insertions(+), 31 deletions(-)
hw/virtio/virtio.c +57 -31
@@ -2954,6 +2954,30 @@ static const VMStateDescription vmstate_virtio_disabled = { } }; +static bool virtio_128bit_features_needed (void *opaque) +{ +VirtIODevice *vdev = opaque; + +return virtio_features_use_ex (vdev->host_features_ex); +} + +static const VMStateDescription vmstate_virtio_128bit_features = { + .name ="virtio/128bit_features" , + .version_id =1 , + .minimum_version_id =1 , + .needed = &virtio_128bit_features_needed, + .fields = (const VMStateField []) { +VMSTATE_UINT64 (guest_features_ex[1 ], VirtIODevice), +VMSTATE_END_OF_LIST () + } +}; + +/* +* Avoid silently breaking migration should the feature space increase +* even more in the (far away) future +*/ +QEMU_BUILD_BUG_ON (VIRTIO_FEATURES_NU64S !=2 ); +static const VMStateDescription vmstate_virtio = { .name ="virtio" , .version_id =1 ,
@@ -2963,6 +2987,7 @@ static const VMStateDescription vmstate_virtio = { }, .subsections = (const VMStateDescription * const []) { &vmstate_virtio_device_endian, + &vmstate_virtio_128bit_features, &vmstate_virtio_64bit_features, &vmstate_virtio_virtqueues, &vmstate_virtio_ringsize,
@@ -3059,23 +3084,28 @@ const VMStateInfo virtio_vmstate_info = { .put = virtio_device_put, }; -static int virtio_set_features_nocheck (VirtIODevice *vdev,uint64_t val) +static int virtio_set_features_nocheck (VirtIODevice *vdev,const uint64_t *val) {VirtioDeviceClass *k =VIRTIO_DEVICE_GET_CLASS (vdev); -bool bad = (val & ~(vdev->host_features)) !=0 ; +uint64_t tmp[VIRTIO_FEATURES_NU64S ]; +bool bad; + + bad =virtio_features_andnot (tmp, val, vdev->host_features_ex); +virtio_features_and (tmp, val, vdev->host_features_ex); - val &= vdev->host_features;if (k->set_features) { - k->set_features (vdev, val); + bad = bad ||virtio_features_use_ex (tmp); + k->set_features (vdev, tmp[0 ]); } - vdev->guest_features = val; + +virtio_features_copy (vdev->guest_features_ex, tmp);return bad ?-1 :0 ; }typedef struct VirtioSetFeaturesNocheckData {Coroutine *co;VirtIODevice *vdev; -uint64_t val; +uint64_t val[VIRTIO_FEATURES_NU64S ];int ret; }VirtioSetFeaturesNocheckData ;
@@ -3088,14 +3118,15 @@ static void virtio_set_features_nocheck_bh(void *opaque) } static int coroutine_mixed_fn -virtio_set_features_nocheck_maybe_co (VirtIODevice *vdev,uint64_t val) +virtio_set_features_nocheck_maybe_co (VirtIODevice *vdev, +const uint64_t *val) {if (qemu_in_coroutine ()) {VirtioSetFeaturesNocheckData data = { .co =qemu_coroutine_self (), .vdev = vdev, - .val = val, }; +virtio_features_copy (data.val, val);aio_bh_schedule_oneshot (qemu_get_current_aio_context (), virtio_set_features_nocheck_bh, &data);qemu_coroutine_yield ();
@@ -3107,6 +3138,7 @@ virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, uint64_t val)int virtio_set_features (VirtIODevice *vdev,uint64_t val) { +uint64_t features[VIRTIO_FEATURES_NU64S ];int ret;/* * The driver must not attempt to set features after feature negotiation
@@ -3122,7 +3154,8 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) __func__, vdev->name); } - ret =virtio_set_features_nocheck (vdev, val); +virtio_features_from_u64 (features, val); + ret =virtio_set_features_nocheck (vdev, features);if (virtio_vdev_has_feature (vdev,VIRTIO_RING_F_EVENT_IDX )) {/* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */ int i;
@@ -3145,6 +3178,7 @@ void virtio_reset(void *opaque) {VirtIODevice *vdev = opaque;VirtioDeviceClass *k =VIRTIO_DEVICE_GET_CLASS (vdev); +uint64_t features[VIRTIO_FEATURES_NU64S ];int i;virtio_set_status (vdev,0 );
@@ -3171,7 +3205,8 @@ void virtio_reset(void *opaque) vdev->start_on_kick = false; vdev->started = false; vdev->broken = false; -virtio_set_features_nocheck (vdev,0 ); +virtio_features_clear (features); +virtio_set_features_nocheck (vdev, features); vdev->queue_sel =0 ; vdev->status =0 ; vdev->disabled = false;
@@ -3254,7 +3289,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)* Note: devices should always test host features in future - don't create * new dependencies like this. */ - vdev->guest_features = features; +virtio_features_from_u64 (vdev->guest_features_ex, features); config_len =qemu_get_be32 (f);
@@ -3333,26 +3368,17 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) vdev->device_endian =virtio_default_endian (); } -if (virtio_64bit_features_needed (vdev)) { -/* -* Subsection load filled vdev->guest_features. Run them -* through virtio_set_features to sanity-check them against -* host_features. -*/ -uint64_t features64 = vdev->guest_features; -if (virtio_set_features_nocheck_maybe_co (vdev, features64) <0 ) { -error_report ("Features 0x%" PRIx64" unsupported. " -"Allowed features: 0x%" PRIx64, - features64, vdev->host_features); -return -1 ; - } - }else { -if (virtio_set_features_nocheck_maybe_co (vdev, features) <0 ) { -error_report ("Features 0x%x unsupported. " -"Allowed features: 0x%" PRIx64, - features, vdev->host_features); -return -1 ; - } +/* +* guest_features_ex is fully initialized with u32 features and upper +* bits have been filled as needed by the later load. +*/ +if (virtio_set_features_nocheck_maybe_co (vdev, + vdev->guest_features_ex) <0 ) { +error_report ("Features 0x" VIRTIO_FEATURES_FMT " unsupported. " +"Allowed features: 0x" VIRTIO_FEATURES_FMT , +VIRTIO_FEATURES_PR (vdev->guest_features_ex), +VIRTIO_FEATURES_PR (vdev->host_features_ex)); +return -1 ; } if (!virtio_device_started(vdev, vdev->status) &&
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 6/14 virtio: add support for negotiating extended features
In reply to Paolo AbeniThe virtio specifications allows for up to 128 bits for the device features. Soon we are going to use some of the 'extended' bits features for the virtio net driver.
Add support to allow extended features negotiation on a per devices basis. Devices willing to negotiated extended features need to implemented a new pair of features getter/setter, the core will conditionally use them instead of the basic one.
Note that 'bad_features' don't need to be extended, as they are bound to the 64 bits limit.
---
v3 -> v4:
- use new virtio_features macro names
v2 -> v3:
- _array -> _ex
v1 -> v2:
- uint128_t -> uint64_t[]
--- hw/virtio/virtio-bus.c | 11 ++++++++--- hw/virtio/virtio.c | 18 +++++++++++++----- include/hw/virtio/virtio.h | 4 ++++ 3 files changed, 25 insertions(+), 8 deletions(-)
hw/virtio/virtio-bus.c +8 -3
@@ -62,9 +62,14 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) }/* Get the features of the plugged device. */ -assert (vdc->get_features !=NULL ); - vdev->host_features = vdc->get_features (vdev, vdev->host_features, - &local_err); +if (vdc->get_features_ex) { + vdc->get_features_ex (vdev, vdev->host_features_ex, &local_err); + }else { +assert (vdc->get_features !=NULL ); +virtio_features_from_u64 (vdev->host_features_ex, + vdc->get_features (vdev, vdev->host_features, + &local_err)); + }if (local_err) {error_propagate (errp, local_err);return ;
hw/virtio/virtio.c +13 -5
@@ -3093,7 +3093,9 @@ static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val) bad =virtio_features_andnot (tmp, val, vdev->host_features_ex);virtio_features_and (tmp, val, vdev->host_features_ex); -if (k->set_features) { +if (k->set_features_ex) { + k->set_features_ex (vdev, val); + }else if (k->set_features) { bad = bad ||virtio_features_use_ex (tmp); k->set_features (vdev, tmp[0 ]); }
@@ -3136,9 +3138,8 @@ virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, } } -int virtio_set_features (VirtIODevice *vdev,uint64_t val) +int virtio_set_features_ex (VirtIODevice *vdev,const uint64_t *features) { -uint64_t features[VIRTIO_FEATURES_NU64S ];int ret;/* * The driver must not attempt to set features after feature negotiation
@@ -3148,13 +3149,12 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)return -EINVAL ; } -if (val & (1ull <<VIRTIO_F_BAD_FEATURE )) { +if (features[0 ] & (1ull <<VIRTIO_F_BAD_FEATURE )) {qemu_log_mask (LOG_GUEST_ERROR ,"%s: guest driver for %s has enabled UNUSED(30) feature bit!\n" , __func__, vdev->name); } -virtio_features_from_u64 (features, val); ret =virtio_set_features_nocheck (vdev, features);if (virtio_vdev_has_feature (vdev,VIRTIO_RING_F_EVENT_IDX )) {/* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
@@ -3174,6 +3174,14 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)return ret; } +int virtio_set_features (VirtIODevice *vdev,uint64_t val) +{ +uint64_t features[VIRTIO_FEATURES_NU64S ]; + +virtio_features_from_u64 (features, val); +return virtio_set_features_ex (vdev, features); +} +void virtio_reset (void *opaque) {VirtIODevice *vdev = opaque;
include/hw/virtio/virtio.h +4 -0
@@ -178,6 +178,9 @@ struct VirtioDeviceClass {/* This is what a VirtioDevice must implement */ DeviceRealize realize;DeviceUnrealize unrealize; +void (*get_features_ex)(VirtIODevice *vdev,uint64_t *requested_features, +Error **errp); +void (*set_features_ex)(VirtIODevice *vdev,const uint64_t *val);uint64_t (*get_features)(VirtIODevice *vdev,uint64_t requested_features,Error **errp);
@@ -373,6 +376,7 @@ void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);void virtio_queue_enable (VirtIODevice *vdev,uint32_t queue_index);void virtio_update_irq (VirtIODevice *vdev);int virtio_set_features (VirtIODevice *vdev,uint64_t val); +int virtio_set_features_ex (VirtIODevice *vdev,const uint64_t *val);/* Base devices. */ typedef struct VirtIOBlkConf VirtIOBlkConf ;
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 10/14 vhost-backend: implement extended features support
In reply to Paolo AbeniLeverage the kernel extended features manipulation ioctls(), if available, and fallback to old ops otherwise. Error out when setting extended features but kernel support is not available.
Note that extended support for get/set backend features is not needed, as the only feature that can be changed belongs to the 64 bit range.
---
v3 -> v4:
- use new virtio_features macro names
v1 -> v2:
- synced with kernel ioctl changes
--- hw/virtio/vhost-backend.c | 62 ++++++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 11 deletions(-)
hw/virtio/vhost-backend.c +51 -11
@@ -20,6 +20,11 @@#include <linux/vhost.h> #include <sys/ioctl.h> +struct vhost_features { +uint64_t count; +uint64_t features[VIRTIO_FEATURES_NU64S ]; +}; +static int vhost_kernel_call (struct vhost_dev *dev,unsigned long int request,void *arg) {
@@ -182,12 +187,6 @@ static int vhost_kernel_get_vring_worker(struct vhost_dev *dev,return vhost_kernel_call (dev,VHOST_GET_VRING_WORKER , worker); } -static int vhost_kernel_set_features (struct vhost_dev *dev, -uint64_t features) -{ -return vhost_kernel_call (dev,VHOST_SET_FEATURES , &features); -} -static int vhost_kernel_set_backend_cap (struct vhost_dev *dev) {uint64_t features;
@@ -210,10 +209,51 @@ static int vhost_kernel_set_backend_cap(struct vhost_dev *dev)return 0 ; } -static int vhost_kernel_get_features (struct vhost_dev *dev, -uint64_t *features) +static int vhost_kernel_set_features (struct vhost_dev *dev, +const uint64_t *features) { -return vhost_kernel_call (dev,VHOST_GET_FEATURES , features); +struct vhost_features farray; +bool extended_in_use; +int r; + + farray.count =VIRTIO_FEATURES_NU64S ; +virtio_features_copy (farray.features, features); + extended_in_use =virtio_features_use_ex (farray.features); + +/* +* Can't check for ENOTTY: for unknown ioctls the kernel interprets +* the argument as a virtio queue id and most likely errors out validating +* such id, instead of reporting an unknown operation. +*/ + r =vhost_kernel_call (dev,VHOST_SET_FEATURES_ARRAY , &farray); +if (!r) { +return 0 ; + } + +if (extended_in_use) { +error_report ("Trying to set extended features without kernel support" ); +return -EINVAL ; + } +return vhost_kernel_call (dev,VHOST_SET_FEATURES , &farray.features[0 ]); +} + +static int vhost_kernel_get_features (struct vhost_dev *dev,uint64_t *features) +{ +struct vhost_features farray; +int r; + + farray.count =VIRTIO_FEATURES_NU64S ; + r =vhost_kernel_call (dev,VHOST_GET_FEATURES_ARRAY , &farray); +if (r) { +memset (&farray,0 ,sizeof (farray)); + r =vhost_kernel_call (dev,VHOST_GET_FEATURES , &farray.features[0 ]); + } +if (r) { +return r; + } + +virtio_features_copy (features, farray.features); +return 0 ; }static int vhost_kernel_set_owner (struct vhost_dev *dev)
@@ -341,8 +381,8 @@ const VhostOps kernel_ops = { .vhost_attach_vring_worker = vhost_kernel_attach_vring_worker, .vhost_new_worker = vhost_kernel_new_worker, .vhost_free_worker = vhost_kernel_free_worker, - .vhost_set_features = vhost_kernel_set_features, - .vhost_get_features = vhost_kernel_get_features, + .vhost_set_features_ex = vhost_kernel_set_features, + .vhost_get_features_ex = vhost_kernel_get_features, .vhost_set_backend_cap = vhost_kernel_set_backend_cap, .vhost_set_owner = vhost_kernel_set_owner, .vhost_get_vq_index = vhost_kernel_get_vq_index,
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 14/14 net: implement UDP tunnel features offloading
In reply to Paolo AbeniWhen any host or guest GSO over UDP tunnel offload is enabled the virtio net header includes the additional tunnel-related fields, update the size accordingly.
Push the GSO over UDP tunnel offloads all the way down to the tap device extending the newly introduced NetFeatures struct, and eventually enable the associated features.
As per virtio specification, to convert features bit to offload bit, map the extended features into the reserved range.
Finally, make the vhost backend aware of the exact header layout, to copy it correctly. The tunnel-related field are present if either the guest or the host negotiated any UDP tunnel related feature:
add them to the kernel supported features list, to allow qemu transfer to the backend the needed information.
---
v3 -> v4:
- '|' -> '||'
v2 -> v3:
- rebased on top of "net: Consolidate vhost feature bits into vhost_net structure"
- _array -> _ex
v1 -> v2:
- squashed vhost support into this patch
- dropped tun offload consistency checks; they are implemented in the kernel side
- virtio_has_tnl_hdr ->virtio_has_tunnel_hdr
--- hw/net/virtio-net.c | 34 ++++++++++++++++++++++++++-------- include/net/net.h | 2 ++ net/net.c | 3 ++- net/tap-linux.c | 6 ++++++ net/tap.c | 2 ++ 5 files changed, 38 insertions(+), 9 deletions(-)
hw/net/virtio-net.c +26 -8
@@ -103,6 +103,12 @@#define VIRTIO_NET_F2O_SHIFT (VIRTIO_NET_OFFLOAD_MAP_MIN - \ VIRTIO_NET_FEATURES_MAP_MIN + 64) +static bool virtio_has_tunnel_hdr (const uint64_t *features) +{ +return virtio_has_feature_ex (features,VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO ) || +virtio_has_feature_ex (features,VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO ); +} +static const VirtIOFeature feature_sizes[] = { {.flags =1ULL <<VIRTIO_NET_F_MAC , .end = endof(struct virtio_net_config, mac)},
@@ -659,7 +665,8 @@ static bool peer_has_tunnel(VirtIONet *n) }static void virtio_net_set_mrg_rx_bufs (VirtIONet *n,int mergeable_rx_bufs, -int version_1,int hash_report) +int version_1,int hash_report, +int tunnel) {int i;NetClientState *nc;
@@ -667,9 +674,11 @@ static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs, n->mergeable_rx_bufs = mergeable_rx_bufs;if (version_1) { - n->guest_hdr_len = hash_report ? -sizeof (struct virtio_net_hdr_v1_hash ) : -sizeof (struct virtio_net_hdr_mrg_rxbuf ); + n->guest_hdr_len = tunnel ? +sizeof (struct virtio_net_hdr_v1_hash_tunnel ) : + (hash_report ? +sizeof (struct virtio_net_hdr_v1_hash ) : +sizeof (struct virtio_net_hdr_mrg_rxbuf )); n->rss_data.populate_hash = !!hash_report; }else { n->guest_hdr_len = n->mergeable_rx_bufs ?
@@ -803,6 +812,10 @@ static void virtio_net_apply_guest_offloads(VirtIONet *n) .ufo = !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)), .uso4 = !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_USO4)), .uso6 = !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_USO6)), + .tnl = !!(n->curr_guest_offloads & + (1ULL << VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_MAPPED)), + .tnl_csum = !!(n->curr_guest_offloads & + (1ULL <<VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM_MAPPED )), };qemu_set_offload (qemu_get_queue (n->nic)->peer, &ol);
@@ -824,7 +837,9 @@ virtio_net_guest_offloads_by_features(const uint64_t *features) (1ULL <<VIRTIO_NET_F_GUEST_ECN ) | (1ULL <<VIRTIO_NET_F_GUEST_UFO ) | (1ULL <<VIRTIO_NET_F_GUEST_USO4 ) | - (1ULL <<VIRTIO_NET_F_GUEST_USO6 ); + (1ULL <<VIRTIO_NET_F_GUEST_USO6 ) | + (1ULL <<VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_MAPPED ) | + (1ULL <<VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM_MAPPED );return guest_offloads_mask &virtio_net_features_to_offload (features); }
@@ -937,7 +952,8 @@ static void virtio_net_set_features(VirtIODevice *vdev,virtio_has_feature_ex (features,VIRTIO_F_VERSION_1 ),virtio_has_feature_ex (features, - VIRTIO_NET_F_HASH_REPORT)); +VIRTIO_NET_F_HASH_REPORT ), + virtio_has_tunnel_hdr(features)); n->rsc4_enabled =virtio_has_feature_ex (features,VIRTIO_NET_F_RSC_EXT ) &&virtio_has_feature_ex (features,VIRTIO_NET_F_GUEST_TSO4 );
@@ -3160,13 +3176,15 @@ static int virtio_net_post_load_device(void *opaque, int version_id)VirtIONet *n = opaque;VirtIODevice *vdev =VIRTIO_DEVICE (n);int i, link_down; +bool has_tunnel_hdr =virtio_has_tunnel_hdr (vdev->guest_features_ex);trace_virtio_net_post_load_device ();virtio_net_set_mrg_rx_bufs (n, n->mergeable_rx_bufs,virtio_vdev_has_feature (vdev,VIRTIO_F_VERSION_1 ),virtio_vdev_has_feature (vdev, -VIRTIO_NET_F_HASH_REPORT )); +VIRTIO_NET_F_HASH_REPORT ), + has_tunnel_hdr);/* MAC_TABLE_ENTRIES may be different from the saved image */ if (n->mac_table.in_use >MAC_TABLE_ENTRIES ) {
@@ -3986,7 +4004,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->vqs[0 ].tx_waiting =0 ; n->tx_burst = n->net_conf.txburst; -virtio_net_set_mrg_rx_bufs (n,0 ,0 ,0 ); +virtio_net_set_mrg_rx_bufs (n,0 ,0 ,0 ,0 ); n->promisc =1 ;/* for compatibility */ n->mac_table.macs =g_malloc0 (MAC_TABLE_ENTRIES *ETH_ALEN );
include/net/net.h +2 -0
@@ -43,6 +43,8 @@ typedef struct NetOffloads {bool ufo;bool uso4;bool uso6; +bool tnl; +bool tnl_csum; } NetOffloads;#define DEFINE_NIC_PROPERTIES (_state, _conf) \
net/net.c +2 -1
@@ -575,7 +575,8 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len)assert (len ==sizeof (struct virtio_net_hdr_mrg_rxbuf ) || len ==sizeof (struct virtio_net_hdr ) || - len ==sizeof (struct virtio_net_hdr_v1_hash )); + len ==sizeof (struct virtio_net_hdr_v1_hash ) || + len ==sizeof (struct virtio_net_hdr_v1_hash_tunnel )); nc->vnet_hdr_len = len; nc->info->set_vnet_hdr_len (nc, len);
net/tap-linux.c +6 -0
@@ -279,6 +279,12 @@ void tap_fd_set_offload(int fd, const NetOffloads *ol)if (ol->uso6) { offload |=TUN_F_USO6 ; } +if (ol->tnl) { + offload |=TUN_F_UDP_TUNNEL_GSO ; + } +if (ol->tnl_csum) { + offload |=TUN_F_UDP_TUNNEL_GSO_CSUM ; + } }if (ioctl (fd,TUNSETOFFLOAD , offload) !=0 ) {
net/tap.c +2 -0
@@ -62,6 +62,8 @@ static const int kernel_feature_bits[] = {VIRTIO_F_NOTIFICATION_DATA ,VIRTIO_NET_F_RSC_EXT ,VIRTIO_NET_F_HASH_REPORT , +VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO , +VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO , VHOST_INVALID_FEATURE_BIT };
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 13/14 net: implement tunnel probing
In reply to Paolo AbeniTap devices support GSO over UDP tunnel offload. Probe for such feature in a similar manner to other offloads.
GSO over UDP tunnel needs to be enabled in addition to a "plain" offload (TSO or USO).
No need to check separately for the outer header checksum offload:
ithe kernel is going to support both of them or none.
Note that this will requires a few compat entries after that hw_compat_10_1 will be added.
--
v3 -> v4:
- move DEFINE_PROP_FEATURE() in patch 4
v2 -> v3:
- use bool type for tap_probe_has_tunnel()
- rebased on top of 2deec9ab7d ("virtio-net: Move virtio_net_get_features() down")
- _array -> _ex
v1 -> v2:
- peer_has_tunnel return a bool
- move TUN_F definition in net/tun-linux.h
--- hw/net/virtio-net.c | 33 +++++++++++++++++++++++++++++++++ include/net/net.h | 3 +++ net/net.c | 9 +++++++++ net/tap-bsd.c | 5 +++++ net/tap-linux.c | 11 +++++++++++ net/tap-linux.h | 9 +++++++++ net/tap-solaris.c | 5 +++++ net/tap-stub.c | 5 +++++ net/tap.c | 11 +++++++++++ net/tap_int.h | 1 + 10 files changed, 92 insertions(+)
hw/net/virtio-net.c +33 -0
@@ -649,6 +649,15 @@ static int peer_has_uso(VirtIONet *n)return qemu_has_uso (qemu_get_queue (n->nic)->peer); } +static bool peer_has_tunnel (VirtIONet *n) +{ +if (!peer_has_vnet_hdr (n)) { +return false; + } + +return qemu_has_tunnel (qemu_get_queue (n->nic)->peer); +} +static void virtio_net_set_mrg_rx_bufs (VirtIONet *n,int mergeable_rx_bufs,int version_1,int hash_report) {
@@ -3070,6 +3079,13 @@ static void virtio_net_get_features(VirtIODevice *vdev, uint64_t *features,virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_USO4 );virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_USO6 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO ); +virtio_clear_feature_ex (features, +VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM ); +virtio_clear_feature_ex (features, +VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HASH_REPORT ); }
@@ -3083,6 +3099,15 @@ static void virtio_net_get_features(VirtIODevice *vdev, uint64_t *features,virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_USO6 ); } +if (!peer_has_tunnel (n)) { +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO ); +virtio_clear_feature_ex (features, +VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM ); +virtio_clear_feature_ex (features, +VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM ); + } +if (!get_vhost_net (nc->peer)) {if (!use_own_hash) {virtio_clear_feature_ex (features,VIRTIO_NET_F_HASH_REPORT );
@@ -4245,6 +4270,14 @@ static const Property virtio_net_properties[] = { rss_data.specified_hash_types, VIRTIO_NET_HASH_REPORT_UDPv6_EX - 1, ON_OFF_AUTO_AUTO), +DEFINE_PROP_FEATURE ("host_tunnel" , VirtIONet, host_features_ex, +VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO , true), +DEFINE_PROP_FEATURE ("host_tunnel_csum" , VirtIONet, host_features_ex, +VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM , true), +DEFINE_PROP_FEATURE ("guest_tunnel" , VirtIONet, host_features_ex, +VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO , true), +DEFINE_PROP_FEATURE ("guest_tunnel_csum" , VirtIONet, host_features_ex, + VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM, true), };static void virtio_net_class_init (ObjectClass *klass,const void *data)
include/net/net.h +3 -0
@@ -65,6 +65,7 @@ typedef void (NetClientDestructor)(NetClientState *);typedef RxFilterInfo *(QueryRxFilter )(NetClientState *);typedef bool (HasUfo )(NetClientState *);typedef bool (HasUso )(NetClientState *); +typedef bool (HasTunnel )(NetClientState *);typedef bool (HasVnetHdr )(NetClientState *);typedef bool (HasVnetHdrLen )(NetClientState *,int );typedef void (SetOffload )(NetClientState *,const NetOffloads *);
@@ -95,6 +96,7 @@ typedef struct NetClientInfo {NetPoll *poll;HasUfo *has_ufo;HasUso *has_uso; +HasTunnel *has_tunnel;HasVnetHdr *has_vnet_hdr;HasVnetHdrLen *has_vnet_hdr_len;SetOffload *set_offload;
@@ -197,6 +199,7 @@ void qemu_set_info_str(NetClientState *nc,void qemu_format_nic_info_str (NetClientState *nc,uint8_t macaddr[6 ]);bool qemu_has_ufo (NetClientState *nc);bool qemu_has_uso (NetClientState *nc); +bool qemu_has_tunnel (NetClientState *nc);bool qemu_has_vnet_hdr (NetClientState *nc);bool qemu_has_vnet_hdr_len (NetClientState *nc,int len);void qemu_set_offload (NetClientState *nc,const NetOffloads *ol);
net/net.c +9 -0
@@ -522,6 +522,15 @@ bool qemu_has_uso(NetClientState *nc)return nc->info->has_uso (nc); } +bool qemu_has_tunnel (NetClientState *nc) +{ +if (!nc || !nc->info->has_tunnel) { +return false; + } + +return nc->info->has_tunnel (nc); +} +bool qemu_has_vnet_hdr (NetClientState *nc) {if (!nc || !nc->info->has_vnet_hdr) {
net/tap-bsd.c +5 -0
@@ -217,6 +217,11 @@ int tap_probe_has_uso(int fd)return 0 ; } +bool tap_probe_has_tunnel (int fd) +{ +return false; +} +void tap_fd_set_vnet_hdr_len (int fd,int len) { }
net/tap-linux.c +11 -0
@@ -196,6 +196,17 @@ int tap_probe_has_uso(int fd)return 1 ; } +bool tap_probe_has_tunnel (int fd) +{ +unsigned offload; + + offload =TUN_F_CSUM |TUN_F_TSO4 |TUN_F_UDP_TUNNEL_GSO ; +if (ioctl (fd,TUNSETOFFLOAD , offload) <0 ) { +return false; + } +return true; +} +void tap_fd_set_vnet_hdr_len (int fd,int len) {if (ioctl (fd,TUNSETVNETHDRSZ , &len) ==-1 ) {
net/tap-linux.h +9 -0
@@ -53,4 +53,13 @@#define TUN_F_USO4 0x20/* I can handle USO for IPv4 packets */ #define TUN_F_USO6 0x40/* I can handle USO for IPv6 packets */ +/* I can handle TSO/USO for UDP tunneled packets */ +#define TUN_F_UDP_TUNNEL_GSO 0x080 + +/* +* I can handle TSO/USO for UDP tunneled packets requiring csum offload for +* the outer header +*/ +#define TUN_F_UDP_TUNNEL_GSO_CSUM 0x100 +#endif /* QEMU_TAP_LINUX_H */
net/tap-solaris.c +5 -0
@@ -222,6 +222,11 @@ int tap_probe_has_uso(int fd)return 0 ; } +bool tap_probe_has_tunnel (int fd) +{ +return false; +} +void tap_fd_set_vnet_hdr_len (int fd,int len) { }
net/tap-stub.c +5 -0
@@ -52,6 +52,11 @@ int tap_probe_has_uso(int fd)return 0 ; } +bool tap_probe_has_tunnel (int fd) +{ +return false; +} +void tap_fd_set_vnet_hdr_len (int fd,int len) { }
net/tap.c +11 -0
@@ -76,6 +76,7 @@ typedef struct TAPState {bool using_vnet_hdr;bool has_ufo;bool has_uso; +bool has_tunnel;bool enabled;VHostNetState *vhost_net;unsigned host_vnet_hdr_len;
@@ -241,6 +242,14 @@ static bool tap_has_uso(NetClientState *nc)return s->has_uso; } +static bool tap_has_tunnel (NetClientState *nc) +{ +TAPState *s =DO_UPCAST (TAPState, nc, nc); + +assert (nc->info->type ==NET_CLIENT_DRIVER_TAP ); +return s->has_tunnel; +} +static bool tap_has_vnet_hdr (NetClientState *nc) {TAPState *s =DO_UPCAST (TAPState, nc, nc);
@@ -369,6 +378,7 @@ static NetClientInfo net_tap_info = { .cleanup = tap_cleanup, .has_ufo = tap_has_ufo, .has_uso = tap_has_uso, + .has_tunnel = tap_has_tunnel, .has_vnet_hdr = tap_has_vnet_hdr, .has_vnet_hdr_len = tap_has_vnet_hdr_len, .set_offload = tap_set_offload,
@@ -398,6 +408,7 @@ static TAPState *net_tap_fd_init(NetClientState *peer, s->using_vnet_hdr = false; s->has_ufo =tap_probe_has_ufo (s->fd); s->has_uso =tap_probe_has_uso (s->fd); + s->has_tunnel =tap_probe_has_tunnel (s->fd); s->enabled = true;tap_set_offload (&s->nc, &ol);/*
net/tap_int.h +1 -0
@@ -38,6 +38,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);int tap_probe_vnet_hdr (int fd,Error **errp);int tap_probe_has_ufo (int fd);int tap_probe_has_uso (int fd); +bool tap_probe_has_tunnel (int fd);void tap_fd_set_offload (int fd,const NetOffloads *ol);void tap_fd_set_vnet_hdr_len (int fd,int len);int tap_fd_set_vnet_le (int fd,int vnet_is_le);
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 9/14 qmp: update virtio features map to support extended features
In reply to Paolo AbeniExtend the VirtioDeviceFeatures struct with an additional u64 to track unknown features in the 64-127 bit range and decode the full virtio features spaces for vhost and virtio devices.
Also add entries for the soon-to-be-supported virtio net GSO over UDP features.
---
v3 -> v4:
- cleanup unknown features init
- update QMP example and doc accordingly
- use new virtio_features macro names
v2 -> v3:
- unknown-dev-features-dword2 -> unknown-dev-features2
- _array -> _ex
- fixed typos in entries description
v1 -> v2:
- uint128_t -> uint64_t[]
--- hw/virtio/virtio-hmp-cmds.c | 3 +- hw/virtio/virtio-qmp.c | 91 +++++++++++++++++++++++++------------ hw/virtio/virtio-qmp.h | 3 +- qapi/virtio.json | 9 +++- 4 files changed, 74 insertions(+), 32 deletions(-)
hw/virtio/virtio-hmp-cmds.c +2 -1
@@ -74,7 +74,8 @@ static void hmp_virtio_dump_features(Monitor *mon, }if (features->has_unknown_dev_features) { -monitor_printf (mon," unknown-features(0x%016" PRIx64")\n" , +monitor_printf (mon," unknown-features(0x%016" PRIx64"%016" PRIx64")\n" , + features->unknown_dev_features2, features->unknown_dev_features); } }
hw/virtio/virtio-qmp.c +63 -28
216 lines not on this page · raw message
hw/virtio/virtio-qmp.h +2 -1
@@ -18,6 +18,7 @@VirtIODevice *qmp_find_virtio_device (const char *path);VirtioDeviceStatus *qmp_decode_status (uint8_t bitmap);VhostDeviceProtocols *qmp_decode_protocols (uint64_t bitmap); -VirtioDeviceFeatures *qmp_decode_features (uint16_t device_id,uint64_t bitmap); +VirtioDeviceFeatures *qmp_decode_features (uint16_t device_id, +const uint64_t *bitmap);#endif
qapi/virtio.json +7 -2
@@ -247,6 +247,7 @@ # }, # "host-features": { # "unknown-dev-features": 1073741824, +# "unknown-dev-features2": 0, # "dev-features": [], # "transports": [ # "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
@@ -490,14 +491,18 @@ # unique features) # # @unknown-dev-features: Virtio device features bitmap that have not -# been decoded +# been decoded (bits 0-63) +# +# @unknown-dev-features2: Virtio device features bitmap that have not +# been decoded (bits 64-127) (since 10.2) # # Since: 7.2 ## { 'struct': 'VirtioDeviceFeatures', 'data': { 'transports': [ 'str' ], '*dev-features': [ 'str' ], - '*unknown-dev-features': 'uint64' } } + '*unknown-dev-features': 'uint64', + '*unknown-dev-features2': 'uint64' } } ## # @VirtQueueStatus:
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 2/14 linux-headers: deal with counted_by annotation
In reply to Paolo AbeniSuch annotation is present into the kernel uAPI headers since v6.7, and will be used soon by the vhost_type.h. Deal with it just stripping it.
--- scripts/update-linux-headers.sh | 1 + 1 file changed, 1 insertion(+)
scripts/update-linux-headers.sh +1 -0
@@ -90,6 +90,7 @@ cp_portable() { -e 's/<linux\/\([^>]*\)>/"standard-headers\/linux\/\1"/' \ -e "$arch_cmd" \ -e 's/__bitwise//' \ + -e 's/__counted_by(\w*)//' \ -e 's/__attribute__((packed))/QEMU_PACKED/' \ -e 's/__inline__/inline/' \ -e 's/__BITS_PER_LONG/HOST_LONG_BITS/' \
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 7/14 virtio-pci: implement support for extended features
In reply to Paolo AbeniExtend the features configuration space to 128 bits. If the virtio device supports any extended features, allow the common read/write operation to access all of it, otherwise keep exposing only the lower 64 bits.
On migration, save the 128 bit version of the features only if the upper bits are non zero. Relay on reset to clear all the feature space before load.
---
v3 -> v4:
- use new virtio_features macro names
- move the build bug before vmstate_virtio_pci_modern_state_sub
v2 -> v3:
- drop the pre_load/post_load trickery and relay on reset zeroing the features instead.
- avoid union usage, just increase guest_features size and use SUB_ARRAY.
- drop unneeded '!!'
- _array -> _ex
v1 -> v2:
- use separate VMStateDescription and pre/post load to avoid breaking migration
- clear proxy features on device reset
--- hw/virtio/virtio-pci.c | 69 +++++++++++++++++++++++++++++----- include/hw/virtio/virtio-pci.h | 2 +- 2 files changed, 61 insertions(+), 10 deletions(-)
hw/virtio/virtio-pci.c +60 -9
@@ -109,6 +109,29 @@ static const VMStateDescription vmstate_virtio_pci_modern_queue_state = { } }; +static bool virtio_pci_modern_state_features128_needed (void *opaque) +{ +VirtIOPCIProxy *proxy = opaque; +uint32_t features =0 ; +int i; + +for (i =2 ; i <ARRAY_SIZE (proxy->guest_features); ++i) { + features |= proxy->guest_features[i]; + } +return features; +} + +static const VMStateDescription vmstate_virtio_pci_modern_state_features128 = { + .name ="virtio_pci/modern_state/features128" , + .version_id =1 , + .minimum_version_id =1 , + .needed = &virtio_pci_modern_state_features128_needed, + .fields = (const VMStateField []) { +VMSTATE_UINT32_SUB_ARRAY (guest_features, VirtIOPCIProxy,2 ,2 ), +VMSTATE_END_OF_LIST () + } +}; +static bool virtio_pci_modern_state_needed (void *opaque) {VirtIOPCIProxy *proxy = opaque;
@@ -116,6 +139,11 @@ static bool virtio_pci_modern_state_needed(void *opaque)return virtio_pci_modern (proxy); } +/* +* Avoid silently breaking migration should the feature space increase +* even more in the (far away) future +*/ +QEMU_BUILD_BUG_ON (VIRTIO_FEATURES_NU32S !=4 );static const VMStateDescription vmstate_virtio_pci_modern_state_sub = { .name ="virtio_pci/modern_state" , .version_id =1 ,
@@ -124,11 +152,15 @@ static const VMStateDescription vmstate_virtio_pci_modern_state_sub = { .fields = (const VMStateField[]) {VMSTATE_UINT32 (dfselect, VirtIOPCIProxy),VMSTATE_UINT32 (gfselect, VirtIOPCIProxy), -VMSTATE_UINT32_ARRAY (guest_features, VirtIOPCIProxy,2 ), +VMSTATE_UINT32_SUB_ARRAY (guest_features, VirtIOPCIProxy,0 ,2 ),VMSTATE_STRUCT_ARRAY (vqs, VirtIOPCIProxy,VIRTIO_QUEUE_MAX ,0 , vmstate_virtio_pci_modern_queue_state, VirtIOPCIQueue),VMSTATE_END_OF_LIST () + }, + .subsections = (const VMStateDescription *const []) { + &vmstate_virtio_pci_modern_state_features128, +NULL } };
@@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,return virtio_pci_add_mem_cap (proxy, &cap.cap); } +static int virtio_pci_select_max (const VirtIODevice *vdev) +{ +return virtio_features_use_ex (vdev->host_features_ex) ? +VIRTIO_FEATURES_NU32S : +2 ; +} +static uint64_t virtio_pci_common_read (void *opaque,hwaddr addr,unsigned size) {
@@ -1494,18 +1533,21 @@ static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr, val = proxy->dfselect;break ;case VIRTIO_PCI_COMMON_DF : -if (proxy->dfselect <=1 ) { +if (proxy->dfselect <virtio_pci_select_max (vdev)) {VirtioDeviceClass *vdc =VIRTIO_DEVICE_GET_CLASS (vdev); - val = (vdev->host_features & ~vdc->legacy_features) >> - (32 * proxy->dfselect); + val = vdev->host_features_ex[proxy->dfselect >>1 ] >> + (32 * (proxy->dfselect &1 )); +if (proxy->dfselect <=1 ) { + val &= (~vdc->legacy_features) >> (32 * proxy->dfselect); + } }break ;case VIRTIO_PCI_COMMON_GFSELECT : val = proxy->gfselect;break ;case VIRTIO_PCI_COMMON_GF : -if (proxy->gfselect <ARRAY_SIZE (proxy->guest_features)) { +if (proxy->gfselect <virtio_pci_select_max (vdev)) { val = proxy->guest_features[proxy->gfselect]; }break ;
@@ -1588,11 +1630,18 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr, proxy->gfselect = val;break ;case VIRTIO_PCI_COMMON_GF : -if (proxy->gfselect <ARRAY_SIZE (proxy->guest_features)) { +if (proxy->gfselect <virtio_pci_select_max (vdev)) { +uint64_t features[VIRTIO_FEATURES_NU64S ]; +int i; + proxy->guest_features[proxy->gfselect] = val; -virtio_set_features (vdev, - (((uint64_t )proxy->guest_features[1 ]) <<32 ) | - proxy->guest_features[0 ]); +virtio_features_clear (features); +for (i =0 ; i <ARRAY_SIZE (proxy->guest_features); ++i) { +uint64_t cur = proxy->guest_features[i]; + + features[i >>1 ] |= cur << ((i &1 ) *32 ); + } +virtio_set_features_ex (vdev, features); }break ;case VIRTIO_PCI_COMMON_MSIX :
@@ -2311,6 +2360,8 @@ static void virtio_pci_reset(DeviceState *qdev)virtio_bus_reset (bus);msix_unuse_all_vectors (&proxy->pci_dev); +memset (proxy->guest_features,0 ,sizeof (proxy->guest_features)); +for (i =0 ; i <VIRTIO_QUEUE_MAX ; i++) { proxy->vqs[i].enabled =0 ; proxy->vqs[i].reset =0 ;
include/hw/virtio/virtio-pci.h +1 -1
@@ -158,7 +158,7 @@ struct VirtIOPCIProxy {uint32_t nvectors;uint32_t dfselect;uint32_t gfselect; -uint32_t guest_features[2 ]; +uint32_t guest_features[VIRTIO_FEATURES_NU32S ];VirtIOPCIQueue vqs[VIRTIO_QUEUE_MAX ];VirtIOIRQFD *vector_irqfd;
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 8/14 vhost: add support for negotiating extended features
In reply to Paolo AbeniSimilar to virtio infra, vhost core maintains the features status in the full extended format and allows the devices to implement extended version of the getter/setter.
Note that 'protocol_features' are not extended: they are only used by vhost-user, and the latter device is not going to implement extended features soon.
---
v3 -> v4:
- fix compile warning for real :(
- de-duplicate code from vhost_{get,ack}_features and vhost_{get,ack}_features_ex
- use new virtio_features macro names
v2 -> v3:
- fix compile warning
- _array -> _ex
v1 -> v2:
- uint128_t -> uint64_t[]
- add _ex() variant of features manipulation helpers
--- hw/virtio/vhost.c | 68 ++++++++++++++++++++++--------- include/hw/virtio/vhost-backend.h | 6 +++ include/hw/virtio/vhost.h | 56 +++++++++++++++++++++---- 3 files changed, 103 insertions(+), 27 deletions(-)
hw/virtio/vhost.c +48 -20
@@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,static int vhost_dev_set_features (struct vhost_dev *dev,bool enable_log) { -uint64_t features = dev->acked_features; +uint64_t features[VIRTIO_FEATURES_NU64S ];int r; + +virtio_features_copy (features, dev->acked_features_ex);if (enable_log) { - features |=0x1ULL <<VHOST_F_LOG_ALL ; +virtio_add_feature_ex (features,VHOST_F_LOG_ALL ); }if (!vhost_dev_has_iommu (dev)) { - features &= ~(0x1ULL <<VIRTIO_F_IOMMU_PLATFORM ); +virtio_clear_feature_ex (features,VIRTIO_F_IOMMU_PLATFORM ); }if (dev->vhost_ops->vhost_force_iommu) {if (dev->vhost_ops->vhost_force_iommu (dev) == true) { - features |=0x1ULL <<VIRTIO_F_IOMMU_PLATFORM ; +virtio_add_feature_ex (features,VIRTIO_F_IOMMU_PLATFORM ); } } - r = dev->vhost_ops->vhost_set_features (dev, features); + +if (virtio_features_use_ex (features) && + !dev->vhost_ops->vhost_set_features_ex) { + r = -EINVAL ; +VHOST_OPS_DEBUG (r,"extended features without device support" ); + goto out; + } + +if (dev->vhost_ops->vhost_set_features_ex) { + r = dev->vhost_ops->vhost_set_features_ex (dev, features); + }else { + r = dev->vhost_ops->vhost_set_features (dev, features[0 ]); + }if (r <0 ) {VHOST_OPS_DEBUG (r,"vhost_set_features failed" ); goto out;
@@ -1506,12 +1520,27 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) } } +static int vhost_dev_get_features (struct vhost_dev *hdev, +uint64_t *features) +{ +uint64_t features64; +int r; + +if (hdev->vhost_ops->vhost_get_features_ex) { +return hdev->vhost_ops->vhost_get_features_ex (hdev, features); + } + + r = hdev->vhost_ops->vhost_get_features (hdev, &features64); +virtio_features_from_u64 (features, features64); +return r; +} +int vhost_dev_init (struct vhost_dev *hdev,void *opaque,VhostBackendType backend_type,uint32_t busyloop_timeout,Error **errp) { +uint64_t features[VIRTIO_FEATURES_NU64S ];unsigned int used, reserved, limit; -uint64_t features;int i, r, n_initialized_vqs =0 ; hdev->vdev =NULL ;
@@ -1531,7 +1560,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, goto fail; } - r = hdev->vhost_ops->vhost_get_features (hdev, &features); + r =vhost_dev_get_features (hdev, features);if (r <0 ) {error_setg_errno (errp, -r,"vhost_get_features failed" ); goto fail;
@@ -1569,7 +1598,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, } } - hdev->features = features; +virtio_features_copy (hdev->features_ex, features); hdev->memory_listener = (MemoryListener ) { .name ="vhost" ,
@@ -1592,7 +1621,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, };if (hdev->migration_blocker ==NULL ) { -if (!(hdev->features & (0x1ULL <<VHOST_F_LOG_ALL ))) { +if (!virtio_has_feature_ex (hdev->features_ex,VHOST_F_LOG_ALL )) {error_setg (&hdev->migration_blocker,"Migration disabled: vhost lacks VHOST_F_LOG_ALL feature." ); }else if (vhost_dev_log_is_shared (hdev) && !qemu_memfd_alloc_check ()) {
@@ -1857,28 +1886,27 @@ static void vhost_start_config_intr(struct vhost_dev *dev) } } -uint64_t vhost_get_features (struct vhost_dev *hdev,const int *feature_bits, -uint64_t features) +void vhost_get_features_ex (struct vhost_dev *hdev, +const int *feature_bits, +uint64_t *features) {const int *bit = feature_bits; +while (*bit !=VHOST_INVALID_FEATURE_BIT ) { -uint64_t bit_mask = (1ULL << *bit); -if (!(hdev->features & bit_mask)) { - features &= ~bit_mask; +if (!virtio_has_feature_ex (hdev->features_ex, *bit)) { +virtio_clear_feature_ex (features, *bit); } bit++; } -return features; } -void vhost_ack_features (struct vhost_dev *hdev,const int *feature_bits, -uint64_t features) +void vhost_ack_features_ex (struct vhost_dev *hdev,const int *feature_bits, +const uint64_t *features) {const int *bit = feature_bits;while (*bit !=VHOST_INVALID_FEATURE_BIT ) { -uint64_t bit_mask = (1ULL << *bit); -if (features & bit_mask) { - hdev->acked_features |= bit_mask; +if (virtio_has_feature_ex (features, *bit)) { +virtio_add_feature_ex (hdev->acked_features_ex, *bit); } bit++; }
include/hw/virtio/vhost-backend.h +6 -0
@@ -95,6 +95,10 @@ typedef int (*vhost_new_worker_op)(struct vhost_dev *dev, struct vhost_worker_state *worker);typedef int (*vhost_free_worker_op )(struct vhost_dev *dev,struct vhost_worker_state *worker); +typedef int (*vhost_set_features_ex_op )(struct vhost_dev *dev, +const uint64_t *features); +typedef int (*vhost_get_features_ex_op )(struct vhost_dev *dev, +uint64_t *features);typedef int (*vhost_set_features_op )(struct vhost_dev *dev,uint64_t features); typedef int (*vhost_get_features_op)(struct vhost_dev *dev,
@@ -186,6 +190,8 @@ typedef struct VhostOps {vhost_free_worker_op vhost_free_worker;vhost_get_vring_worker_op vhost_get_vring_worker;vhost_attach_vring_worker_op vhost_attach_vring_worker; +vhost_set_features_ex_op vhost_set_features_ex; +vhost_get_features_ex_op vhost_get_features_ex;vhost_set_features_op vhost_set_features;vhost_get_features_op vhost_get_features;vhost_set_backend_cap_op vhost_set_backend_cap;
include/hw/virtio/vhost.h +49 -7
80 lines not on this page · raw message
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 11/14 vhost-net: implement extended features support
In reply to Paolo AbeniProvide extended version of the features manipulation helpers, and let the device initialization deal with the full features space, adjusting the relevant format strings accordingly.
---
v3 -> v4:
- add missing features initialization
- use new virtio_features macro names
v2 -> v3:
- use virtio_features_andnot return value instead of virtio_features_empty
- rebased on top of "net: Consolidate vhost feature bits into vhost_net structure"
- _array -> _ex
v1 -> v2:
- uint128_t -> uint64_t[]
- provide extended variant of the features manipulation helpers
--- hw/net/vhost_net-stub.c | 8 +++----- hw/net/vhost_net.c | 45 +++++++++++++++++++++++------------------ include/net/vhost_net.h | 33 +++++++++++++++++++++++++++--- 3 files changed, 58 insertions(+), 28 deletions(-)
hw/net/vhost_net-stub.c +3 -5
@@ -46,9 +46,8 @@ void vhost_net_cleanup(struct vhost_net *net) { } -uint64_t vhost_net_get_features (struct vhost_net *net,uint64_t features) +void vhost_net_get_features_ex (struct vhost_net *net,uint64_t *features) { -return features; } int vhost_net_get_config(struct vhost_net *net, uint8_t *config,
@@ -62,13 +61,12 @@ int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,return 0 ; } -void vhost_net_ack_features (struct vhost_net *net,uint64_t features) +void vhost_net_ack_features_ex (struct vhost_net *net,const uint64_t *features) { } -uint64_t vhost_net_get_acked_features (VHostNetState *net) +void vhost_net_get_acked_features_ex (VHostNetState *net,uint64_t *features) { -return 0 ; }bool vhost_net_virtqueue_pending (VHostNetState *net,int idx)
hw/net/vhost_net.c +25 -20
100 lines not on this page · raw message
include/net/vhost_net.h +30 -3
@@ -2,6 +2,7 @@#define VHOST_NET_H #include "net/net.h" +#include "hw/virtio/virtio-features.h" #include "hw/virtio/vhost-backend.h" struct vhost_net ;
@@ -33,8 +34,26 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,void vhost_net_cleanup (VHostNetState *net); -uint64_t vhost_net_get_features (VHostNetState *net,uint64_t features); -void vhost_net_ack_features (VHostNetState *net,uint64_t features); +void vhost_net_get_features_ex (VHostNetState *net,uint64_t *features); +static inline uint64_t vhost_net_get_features (VHostNetState *net, +uint64_t features) +{ +uint64_t features_array[VIRTIO_FEATURES_NU64S ]; + +virtio_features_from_u64 (features_array, features); +vhost_net_get_features_ex (net, features_array); +return features_array[0 ]; +} + +void vhost_net_ack_features_ex (VHostNetState *net,const uint64_t *features); +static inline void vhost_net_ack_features (VHostNetState *net, +uint64_t features) +{ +uint64_t features_array[VIRTIO_FEATURES_NU64S ]; + +virtio_features_from_u64 (features_array, features); +vhost_net_ack_features_ex (net, features_array); +}int vhost_net_get_config (struct vhost_net *net,uint8_t *config,uint32_t config_len);
@@ -51,7 +70,15 @@ VHostNetState *get_vhost_net(NetClientState *nc);int vhost_net_set_vring_enable (NetClientState *nc,int enable); -uint64_t vhost_net_get_acked_features (VHostNetState *net); +void vhost_net_get_acked_features_ex (VHostNetState *net,uint64_t *features); +static inline uint64_t vhost_net_get_acked_features (VHostNetState *net) +{ +uint64_t features[VIRTIO_FEATURES_NU64S ]; + +vhost_net_get_acked_features_ex (net, features); +assert (!virtio_features_use_ex (features)); +return features[0 ]; +}int vhost_net_set_mtu (struct vhost_net *net,uint16_t mtu);
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 3/14 linux-headers: Update to Linux ~v6.16-rc7 net-next
In reply to Paolo AbeniUpdate headers to include the virtio GSO over UDP tunnel features
--
Note: while the relevant change are not into Linus's tree yet, they have been merged in the net-next tree and they should land into the vanilla tree during the next merge window.
--- include/standard-headers/linux/ethtool.h | 4 +-- include/standard-headers/linux/vhost_types.h | 5 +++ include/standard-headers/linux/virtio_net.h | 33 ++++++++++++++++++++ linux-headers/linux/vhost.h | 7 +++++ 4 files changed, 47 insertions(+), 2 deletions(-)
include/standard-headers/linux/ethtool.h +2 -2
@@ -2314,7 +2314,7 @@ enum {IPV6_USER_FLOW =0x0e ,/* spec only (usr_ip6_spec; nfc only) */ IPV4_FLOW =0x10 ,/* hash only */ IPV6_FLOW =0x11 ,/* hash only */ -ETHER_FLOW =0x12 ,/* spec only (ether_spec) */ +ETHER_FLOW =0x12 ,/* hash or spec (ether_spec) */ /* Used for GTP-U IPv4 and IPv6. * The format of GTP packets only includes
@@ -2371,7 +2371,7 @@ enum {/* Flag to enable RSS spreading of traffic matching rule (nfc only) */ #define FLOW_RSS 0x20000000 -/* L3-L4 network traffic flow hash options */ +/* L2-L4 network traffic flow hash options */ #define RXH_L2DA (1 << 1)#define RXH_VLAN (1 << 2)#define RXH_L3_PROTO (1 << 3)
include/standard-headers/linux/vhost_types.h +5 -0
@@ -110,6 +110,11 @@ struct vhost_msg_v2 { }; }; +struct vhost_features_array { +uint64_t count;/* number of entries present in features array */ +uint64_t features[] ; +}; +struct vhost_memory_region {uint64_t guest_phys_addr;uint64_t memory_size;/* bytes */
include/standard-headers/linux/virtio_net.h +33 -0
@@ -70,6 +70,28 @@* with the same MAC. */ #define VIRTIO_NET_F_SPEED_DUPLEX 63/* Device set linkspeed and duplex */ +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO 65/* Driver can receive +* GSO-over-UDP-tunnel packets +*/ +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM 66/* Driver handles +* GSO-over-UDP-tunnel +* packets with partial csum +* for the outer header +*/ +#define VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO 67/* Device can receive +* GSO-over-UDP-tunnel packets +*/ +#define VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM 68/* Device handles +* GSO-over-UDP-tunnel +* packets with partial csum +* for the outer header +*/ + +/* Offloads bits corresponding to VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO{,_CSUM} +* features +*/ +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_MAPPED 46 +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM_MAPPED 47#ifndef VIRTIO_NET_NO_LEGACY #define VIRTIO_NET_F_GSO 6/* Host handles pkts w/ any GSO type */
@@ -131,12 +153,17 @@ struct virtio_net_hdr_v1 {#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1/* Use csum_start, csum_offset */ #define VIRTIO_NET_HDR_F_DATA_VALID 2/* Csum is valid */ #define VIRTIO_NET_HDR_F_RSC_INFO 4/* rsc info in csum_ fields */ +#define VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM 8/* UDP tunnel csum offload */ uint8_t flags;#define VIRTIO_NET_HDR_GSO_NONE 0/* Not a GSO frame */ #define VIRTIO_NET_HDR_GSO_TCPV4 1/* GSO frame, IPv4 TCP (TSO) */ #define VIRTIO_NET_HDR_GSO_UDP 3/* GSO frame, IPv4 UDP (UFO) */ #define VIRTIO_NET_HDR_GSO_TCPV6 4/* GSO frame, IPv6 TCP */ #define VIRTIO_NET_HDR_GSO_UDP_L4 5/* GSO frame, IPv4& IPv6 UDP (USO) */ +#define VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 0x20/* UDPv4 tunnel present */ +#define VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 0x40/* UDPv6 tunnel present */ +#define VIRTIO_NET_HDR_GSO_UDP_TUNNEL (VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 | \ + VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6)#define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */ uint8_t gso_type;__virtio16 hdr_len;/* Ethernet + IP + tcp/udp hdrs */
@@ -181,6 +208,12 @@ struct virtio_net_hdr_v1_hash {uint16_t padding; }; +struct virtio_net_hdr_v1_hash_tunnel { +struct virtio_net_hdr_v1_hash hash_hdr; +uint16_t outer_th_offset; +uint16_t inner_nh_offset; +}; +#ifndef VIRTIO_NET_NO_LEGACY /* This header comes first in the scatter-gather list. * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
linux-headers/linux/vhost.h +7 -0
@@ -235,4 +235,11 @@*/ #define VHOST_VDPA_GET_VRING_SIZE _IOWR(VHOST_VIRTIO, 0x82, \ struct vhost_vring_state) + +/* Extended features manipulation */ +#define VHOST_GET_FEATURES_ARRAY _IOR(VHOST_VIRTIO, 0x83, \ + struct vhost_features_array) +#define VHOST_SET_FEATURES_ARRAY _IOW(VHOST_VIRTIO, 0x83, \ + struct vhost_features_array) +#endif
Signature · 1 line
2.50.0
Paolo Abeni
maintainerRFC PATCH v4 12/14 virtio-net: implement extended features support
In reply to Paolo AbeniUse 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.
---
v3 -> v4:
- use new virtio_features macro names
v2 -> v3:
- rebased on top of 2deec9ab7d ("virtio-net: Move virtio_net_get_features() down")
- _array -> _ex
v1 -> v2:
- uint128_t -> uint64_t[]
- more verbose macro definitions
--- hw/net/virtio-net.c | 136 +++++++++++++++++++-------------- include/hw/virtio/virtio-net.h | 2 +- 2 files changed, 80 insertions(+), 58 deletions(-)
hw/net/virtio-net.c +79 -57
@@ -90,6 +90,19 @@ VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | \ VIRTIO_NET_RSS_HASH_TYPE_UDP_EX) +/* +* Features starting from VIRTIO_NET_FEATURES_MAP_MIN bit correspond +* to guest offloads in the VIRTIO_NET_OFFLOAD_MAP range +*/ +#define VIRTIO_NET_OFFLOAD_MAP_MIN 46 +#define VIRTIO_NET_OFFLOAD_MAP_LENGTH 4 +#define VIRTIO_NET_OFFLOAD_MAP MAKE_64BIT_MASK( \ + VIRTIO_NET_OFFLOAD_MAP_MIN, \ + VIRTIO_NET_OFFLOAD_MAP_LENGTH) +#define VIRTIO_NET_FEATURES_MAP_MIN 65 +#define VIRTIO_NET_F2O_SHIFT (VIRTIO_NET_OFFLOAD_MAP_MIN - \ + VIRTIO_NET_FEATURES_MAP_MIN + 64) +static const VirtIOFeature feature_sizes[] = { {.flags =1ULL <<VIRTIO_NET_F_MAC , .end = endof(struct virtio_net_config, mac)},
@@ -786,7 +799,14 @@ static void virtio_net_apply_guest_offloads(VirtIONet *n)qemu_set_offload (qemu_get_queue (n->nic)->peer, &ol); } -static uint64_t virtio_net_guest_offloads_by_features (uint64_t features) +static uint64_t virtio_net_features_to_offload (const uint64_t *features) +{ +return (features[0 ] & ~VIRTIO_NET_OFFLOAD_MAP ) | + ((features[1 ] <<VIRTIO_NET_F2O_SHIFT ) &VIRTIO_NET_OFFLOAD_MAP ); +} + +static uint64_t +virtio_net_guest_offloads_by_features (const uint64_t *features) {static const uint64_t guest_offloads_mask = (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
@@ -797,13 +817,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); }uint64_t virtio_net_supported_guest_offloads (const VirtIONet *n) {VirtIODevice *vdev =VIRTIO_DEVICE (n); -return virtio_net_guest_offloads_by_features (vdev->guest_features); +return virtio_net_guest_offloads_by_features (vdev->guest_features_ex); } typedef struct {
@@ -882,34 +902,39 @@ static void failover_add_primary(VirtIONet *n, Error **errp)error_propagate (errp, err); } -static void virtio_net_set_features (VirtIODevice *vdev,uint64_t features) +static void virtio_net_set_features (VirtIODevice *vdev, +const uint64_t *in_features) { +uint64_t features[VIRTIO_FEATURES_NU64S ];VirtIONet *n =VIRTIO_NET (vdev);Error *err =NULL ;int i; +virtio_features_copy (features, in_features);if (n->mtu_bypass_backend && !virtio_has_feature (vdev->backend_features,VIRTIO_NET_F_MTU )) { - features &= ~(1ULL <<VIRTIO_NET_F_MTU ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_MTU ); }virtio_net_set_multiqueue (n, -virtio_has_feature (features,VIRTIO_NET_F_RSS ) || -virtio_has_feature (features,VIRTIO_NET_F_MQ )); +virtio_has_feature_ex (features, +VIRTIO_NET_F_RSS ) || +virtio_has_feature_ex (features, +VIRTIO_NET_F_MQ ));virtio_net_set_mrg_rx_bufs (n, -virtio_has_feature (features, +virtio_has_feature_ex (features,VIRTIO_NET_F_MRG_RXBUF ), -virtio_has_feature (features, +virtio_has_feature_ex (features,VIRTIO_F_VERSION_1 ), -virtio_has_feature (features, +virtio_has_feature_ex (features,VIRTIO_NET_F_HASH_REPORT )); - n->rsc4_enabled =virtio_has_feature (features,VIRTIO_NET_F_RSC_EXT ) && -virtio_has_feature (features,VIRTIO_NET_F_GUEST_TSO4 ); - n->rsc6_enabled =virtio_has_feature (features,VIRTIO_NET_F_RSC_EXT ) && -virtio_has_feature (features,VIRTIO_NET_F_GUEST_TSO6 ); - n->rss_data.redirect =virtio_has_feature (features,VIRTIO_NET_F_RSS ); + n->rsc4_enabled =virtio_has_feature_ex (features,VIRTIO_NET_F_RSC_EXT ) && +virtio_has_feature_ex (features,VIRTIO_NET_F_GUEST_TSO4 ); + n->rsc6_enabled =virtio_has_feature_ex (features,VIRTIO_NET_F_RSC_EXT ) && +virtio_has_feature_ex (features,VIRTIO_NET_F_GUEST_TSO6 ); + n->rss_data.redirect =virtio_has_feature_ex (features,VIRTIO_NET_F_RSS );if (n->has_vnet_hdr) { n->curr_guest_offloads =
@@ -923,7 +948,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)if (!get_vhost_net (nc->peer)) {continue ; } -vhost_net_ack_features (get_vhost_net (nc->peer), features); +vhost_net_ack_features_ex (get_vhost_net (nc->peer), features);/* * keep acked_features in NetVhostUserState up-to-date so it
@@ -932,11 +957,11 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)vhost_net_save_acked_features (nc->peer); } -if (!virtio_has_feature (features,VIRTIO_NET_F_CTRL_VLAN )) { +if (!virtio_has_feature_ex (features,VIRTIO_NET_F_CTRL_VLAN )) {memset (n->vlans,0xff ,MAX_VLAN >>3 ); } -if (virtio_has_feature (features,VIRTIO_NET_F_STANDBY )) { +if (virtio_has_feature_ex (features,VIRTIO_NET_F_STANDBY )) {qapi_event_send_failover_negotiated (n->netclient_name);qatomic_set (&n->failover_primary_hidden, false);failover_add_primary (n, &err);
@@ -1901,10 +1926,10 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,virtio_error (vdev,"virtio-net unexpected empty queue: " "i %zd mergeable %d offset %zd, size %zd, " "guest hdr len %zd, host hdr len %zd " -"guest features 0x%" PRIx64, +"guest features 0x" VIRTIO_FEATURES_FMT , i, n->mergeable_rx_bufs, offset, size, n->guest_hdr_len, n->host_hdr_len, - vdev->guest_features); +VIRTIO_FEATURES_PR (vdev->guest_features_ex)); } err =-1 ; goto err;
@@ -3011,8 +3036,8 @@ static int virtio_net_pre_load_queues(VirtIODevice *vdev, uint32_t n)return 0 ; } -static uint64_t virtio_net_get_features (VirtIODevice *vdev,uint64_t features, -Error **errp) +static void virtio_net_get_features (VirtIODevice *vdev,uint64_t *features, +Error **errp) {VirtIONet *n =VIRTIO_NET (vdev);NetClientState *nc =qemu_get_queue (n->nic);
@@ -3026,68 +3051,67 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features, (supported_hash_types & peer_hash_types) == supported_hash_types;/* Firstly sync all virtio-net possible supported features */ - features |= n->host_features; +virtio_features_or (features, features, n->host_features_ex); -virtio_add_feature (&features,VIRTIO_NET_F_MAC ); +virtio_add_feature_ex (features,VIRTIO_NET_F_MAC );if (!peer_has_vnet_hdr (n)) { -virtio_clear_feature (&features,VIRTIO_NET_F_CSUM ); -virtio_clear_feature (&features,VIRTIO_NET_F_HOST_TSO4 ); -virtio_clear_feature (&features,VIRTIO_NET_F_HOST_TSO6 ); -virtio_clear_feature (&features,VIRTIO_NET_F_HOST_ECN ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_CSUM ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HOST_TSO4 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HOST_TSO6 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HOST_ECN ); -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_CSUM ); -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_TSO4 ); -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_TSO6 ); -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_ECN ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_CSUM ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_TSO4 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_TSO6 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_ECN ); -virtio_clear_feature (&features,VIRTIO_NET_F_HOST_USO ); -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_USO4 ); -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_USO6 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HOST_USO ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_USO4 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_USO6 ); -virtio_clear_feature (&features,VIRTIO_NET_F_HASH_REPORT ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HASH_REPORT ); }if (!peer_has_vnet_hdr (n) || !peer_has_ufo (n)) { -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_UFO ); -virtio_clear_feature (&features,VIRTIO_NET_F_HOST_UFO ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_UFO ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HOST_UFO ); } -if (!peer_has_uso (n)) { -virtio_clear_feature (&features,VIRTIO_NET_F_HOST_USO ); -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_USO4 ); -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_USO6 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HOST_USO ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_USO4 ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_USO6 ); }if (!get_vhost_net (nc->peer)) {if (!use_own_hash) { -virtio_clear_feature (&features,VIRTIO_NET_F_HASH_REPORT ); -virtio_clear_feature (&features,VIRTIO_NET_F_RSS ); - }else if (virtio_has_feature (features,VIRTIO_NET_F_RSS )) { +virtio_clear_feature_ex (features,VIRTIO_NET_F_HASH_REPORT ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_RSS ); + }else if (virtio_has_feature_ex (features,VIRTIO_NET_F_RSS )) {virtio_net_load_ebpf (n, errp); } -return features; +return ; }if (!use_peer_hash) { -virtio_clear_feature (&features,VIRTIO_NET_F_HASH_REPORT ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_HASH_REPORT );if (!use_own_hash || !virtio_net_attach_ebpf_to_backend (n->nic,-1 )) {if (!virtio_net_load_ebpf (n, errp)) { -return features; +return ; } -virtio_clear_feature (&features,VIRTIO_NET_F_RSS ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_RSS ); } } - features =vhost_net_get_features (get_vhost_net (nc->peer), features); - vdev->backend_features = features; +vhost_net_get_features_ex (get_vhost_net (nc->peer), features); +virtio_features_copy (vdev->backend_features_ex, features);if (n->mtu_bypass_backend && (n->host_features &1ULL <<VIRTIO_NET_F_MTU )) { - features |= (1ULL <<VIRTIO_NET_F_MTU ); +virtio_add_feature_ex (features,VIRTIO_NET_F_MTU ); }/*
@@ -3102,10 +3126,8 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,* support it. */ if (!virtio_has_feature (vdev->backend_features,VIRTIO_NET_F_CTRL_VQ )) { -virtio_clear_feature (&features,VIRTIO_NET_F_GUEST_ANNOUNCE ); +virtio_clear_feature_ex (features,VIRTIO_NET_F_GUEST_ANNOUNCE ); } - -return features; }static int virtio_net_post_load_device (void *opaque,int version_id)
@@ -4237,8 +4259,8 @@ static void virtio_net_class_init(ObjectClass *klass, const void *data) vdc->unrealize = virtio_net_device_unrealize; vdc->get_config = virtio_net_get_config; vdc->set_config = virtio_net_set_config; - vdc->get_features = virtio_net_get_features; - vdc->set_features = virtio_net_set_features; + vdc->get_features_ex = virtio_net_get_features; + vdc->set_features_ex = virtio_net_set_features; vdc->bad_features = virtio_net_bad_features; vdc->reset = virtio_net_reset; vdc->queue_reset = virtio_net_queue_reset;
include/hw/virtio/virtio-net.h +1 -1
@@ -182,7 +182,7 @@ struct VirtIONet {uint32_t has_vnet_hdr;size_t host_hdr_len;size_t guest_hdr_len; -uint64_t host_features; +VIRTIO_DECLARE_FEATURES (host_features);uint32_t rsc_timeout;uint8_t rsc4_enabled;uint8_t rsc6_enabled;
Signature · 1 line
2.50.0
Re: [RFC PATCH v4 04/14] virtio: introduce extended features type
In reply to Paolo AbeniQuoted 64 lines · Paolo Abeni
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 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.
Signed-off-by Paolo Abeni <[email protected]>---
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
- moveDEFINE_PROP_FEATUREdefinition herev2 -> v3:
- fix preprocessor guard name
- useBIT_ULL
- add missing parentheses
- usememcmp()
-_is_empty()->_empty()
-_andnot()returns a bool, true if dst has any non zero bits
-_array->_exv1 -> 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 | 127 ++++++++++++++++++++++++++++ include/hw/virtio/virtio.h | 7 +- 2 files changed, 131 insertions(+), 3 deletions(-)create mode 100644
include/hw/virtio/virtio-features.hinclude/hw/virtio/virtio-features.h new file +19
-0@@ -0,0 +1,127 @@ +/* +* 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)
include/hw/virtio/virtio-features.h line 29
Quoted 10 lines
include/hw/virtio/virtio-features.h new file +10
-0+#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 DEFINE_PROP_FEATURE (_name, _state, _field, _bit, _defval) \
This should contain "VIRTIO" in its name just like other identifiers in this file.
Quoted 123 lines
include/hw/virtio/virtio-features.h new file +98
-0+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 +include/hw/virtio/virtio.h +4
-3@@ -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;
Re: [RFC PATCH v4 06/14] virtio: add support for negotiating extended features
In reply to Paolo AbeniQuoted 93 lines · Paolo Abeni
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 for the virtio net driver.
Add support to allow extended features negotiation on a per devices basis. Devices willing to negotiated extended features need to implemented a new pair of features getter/setter, the core will conditionally use them instead of the basic one.
Note that '
bad_features' don't need to be extended, as they are bound to the 64 bits limit.Signed-off-by Paolo Abeni <[email protected]>---
v3 -> v4:
- use newvirtio_featuresmacro namesv2 -> v3:
-_array->_exv1 -> v2:
-uint128_t-> uint64_t[]--- hw/virtio/virtio-bus.c | 11 ++++++++--- hw/virtio/virtio.c | 18 +++++++++++++----- include/hw/virtio/virtio.h | 4 ++++ 3 files changed, 25 insertions(+), 8 deletions(-)hw/virtio/virtio-bus.c +8
-3@@ -62,9 +62,14 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) }/* Get the features of the plugged device. */ -assert (vdc->get_features !=NULL ); - vdev->host_features = vdc->get_features (vdev, vdev->host_features, - &local_err); +if (vdc->get_features_ex) { + vdc->get_features_ex (vdev, vdev->host_features_ex, &local_err); + }else { +assert (vdc->get_features !=NULL ); +virtio_features_from_u64 (vdev->host_features_ex, + vdc->get_features (vdev, vdev->host_features, + &local_err)); + }if (local_err) {error_propagate (errp, local_err);return ;hw/virtio/virtio.c +5
-5@@ -3093,7 +3093,9 @@ static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val) bad =virtio_features_andnot (tmp, val, vdev->host_features_ex);virtio_features_and (tmp, val, vdev->host_features_ex); -if (k->set_features) { +if (k->set_features_ex) { + k->set_features_ex (vdev, val); + }else if (k->set_features) { bad = bad ||virtio_features_use_ex (tmp); k->set_features (vdev, tmp[0 ]); }@@ -3136,9 +3138,8 @@ virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, } } -int virtio_set_features (VirtIODevice *vdev,uint64_t val) +int virtio_set_features_ex (VirtIODevice *vdev,const uint64_t *features) { -uint64_t features[VIRTIO_FEATURES_NU64S ];int ret;/* * The driver must not attempt to set features after feature negotiation @@ -3148,13 +3149,12 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)return -EINVAL ; } -if (val & (1ull <<VIRTIO_F_BAD_FEATURE )) { +if (features[0 ] & (1ull <<VIRTIO_F_BAD_FEATURE )) {qemu_log_mask (LOG_GUEST_ERROR ,"%s: guest driver for %s has enabled UNUSED(30) feature bit!\n" , __func__, vdev->name); } -virtio_features_from_u64 (features, val); ret =virtio_set_features_nocheck (vdev, features);if (virtio_vdev_has_feature (vdev,VIRTIO_RING_F_EVENT_IDX )) {/* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */ @@ -3174,6 +3174,14 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)return ret;
hw/virtio/virtio.c int virtio_set_features(VirtIODevice *vdev, uint64_t val) · line 3184
Quoted 10 lines
hw/virtio/virtio.c +8
-0} +int virtio_set_features (VirtIODevice *vdev,uint64_t val) +{ +uint64_t features[VIRTIO_FEATURES_NU64S ]; + +virtio_features_from_u64 (features, val); +return virtio_set_features_ex (vdev, features); +} +
Nitpick: this patch will be smaller by a few lines if you move virtio_set_features() before virtio_set_features_ex(). It will also align the order of these functions with the header file.
Quoted 25 lines
hw/virtio/virtio.c +0
-0void virtio_reset (void *opaque) {VirtIODevice *vdev = opaque;include/hw/virtio/virtio.h +4
-0@@ -178,6 +178,9 @@ struct VirtioDeviceClass {/* This is what a VirtioDevice must implement */ DeviceRealize realize;DeviceUnrealize unrealize; +void (*get_features_ex)(VirtIODevice *vdev,uint64_t *requested_features, +Error **errp); +void (*set_features_ex)(VirtIODevice *vdev,const uint64_t *val);uint64_t (*get_features)(VirtIODevice *vdev,uint64_t requested_features,Error **errp);@@ -373,6 +376,7 @@ void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);void virtio_queue_enable (VirtIODevice *vdev,uint32_t queue_index);void virtio_update_irq (VirtIODevice *vdev);int virtio_set_features (VirtIODevice *vdev,uint64_t val); +int virtio_set_features_ex (VirtIODevice *vdev,const uint64_t *val);/* Base devices. */ typedef struct VirtIOBlkConf VirtIOBlkConf ;
Tested this series of patches V4 with virtio-net regression tests, everything works fine.
Quoted 117 lines · Paolo Abeni
Some virtualized deployments use UDP tunnel pervasively and are impacted negatively by the lack of GSO support for such kind of traffic in the virtual NIC driver.
The
virtio_netspecification recently introduced support for GSO over UDP tunnel, and the kernel side of the implementation has been merged into the net-next tree; this series updates the virtio implementation to support such a feature.Currently the qemu virtio support limits the feature space to 64 bits, while the virtio specification allows for a larger number of features. Specifically the GSO-over-UDP-tunnel-related virtio features use bits 65-69; the larger part of this series (patches 4-12) actually deals with extending the features space.
The extended features are carried by fixed size
uint64_tarrays, bringing the current maximum features number to 128.The patches use some syntactic sugar to try to minimize the otherwise very large code churn. Specifically the extended features are boundled in an union with 'legacy' features definition, allowing no changes in the virtio devices not needing the extended features set.
The actual offload implementation is in patches 13 and 14 and boils down to propagating the new offload to the tun devices and the vhost backend.
Patch 1 is a small pre-req refactor that ideally could enter the tree separately; it's presented here in the same series to help reviewers more easily getting the full picture, patch 2 updates to linux headers update script to deal with annotations recently introduce in the kernel and patch 3 is a needed linux headers update.
Tested with basic stream transfer with all the possible permutations of host
kernel/qemu/guestkernel with/without GSO over UDP tunnel support, vs snapshots creation and restore and vs migration.Sharing again as RFC as the kernel bits have not entered the Linus tree yet - but they should on next merge window.
---
Note: next revision will be after the Linux merge window to allow dropping the 'RFC' tag and also due to myself being off-the-grid meanwhile.v3 -> v4:
- more sparse cleanups
- fix build failures on a couple of targetsv2 -> v3:
- consolidated suffixes for new fields to '_ex'
- avoid pre/post load trickery and relay on reset zeroing the features
- cleaned-up virtio store implementation deduplicating a bit of code
- many more cleanups, see the individual patches changelog for the details
- I left patch 1 unmodified, still some hope we could live with that;)Paolo Abeni (14):
net: bundle all offloads in a single struct
linux-headers: deal withcounted_byannotation
linux-headers: Update to Linux ~v6.16-rc7 net-next
virtio: introduce extended features type
virtio: serialize extended features state
virtio: add support for negotiating extended features
virtio-pci: implement support for extended features
vhost: add support for negotiating extended features
qmp: update virtio features map to support extended features
vhost-backend: implement extended features support
vhost-net: implement extended features support
virtio-net: implement extended features support
net: implement tunnel probing
net: implement UDP tunnel features offloadinghw/net/e1000e_core.c | 5 +- hw/net/igb_core.c | 5 +- hw/net/vhost_net-stub.c | 8 +- hw/net/vhost_net.c | 45 ++-- hw/net/virtio-net.c | 222 +++++++++++++------ hw/net/vmxnet3.c | 13 +- hw/virtio/vhost-backend.c | 62 +++++- hw/virtio/vhost.c | 68 ++++-- hw/virtio/virtio-bus.c | 11 +- hw/virtio/virtio-hmp-cmds.c | 3 +- hw/virtio/virtio-pci.c | 69 +++++- hw/virtio/virtio-qmp.c | 91 +++++--- hw/virtio/virtio-qmp.h | 3 +- hw/virtio/virtio.c | 102 ++++++--- include/hw/virtio/vhost-backend.h | 6 + include/hw/virtio/vhost.h | 56 ++++- include/hw/virtio/virtio-features.h | 127 +++++++++++ include/hw/virtio/virtio-net.h | 2 +- include/hw/virtio/virtio-pci.h | 2 +- include/hw/virtio/virtio.h | 11 +- include/net/net.h | 20 +- include/net/vhost_net.h | 33 ++- include/standard-headers/linux/ethtool.h | 4 +- include/standard-headers/linux/vhost_types.h | 5 + include/standard-headers/linux/virtio_net.h | 33 +++ linux-headers/linux/vhost.h | 7 + net/net.c | 17 +- net/netmap.c | 3 +- net/tap-bsd.c | 8 +- net/tap-linux.c | 38 +++- net/tap-linux.h | 9 + net/tap-solaris.c | 9 +- net/tap-stub.c | 8 +- net/tap.c | 21 +- net/tap_int.h | 5 +- qapi/virtio.json | 9 +- scripts/update-linux-headers.sh | 1 + 37 files changed, 875 insertions(+), 266 deletions(-) create mode 100644 include/hw/virtio/virtio-features.h--
2.50.0
Re: [RFC PATCH v4 07/14] virtio-pci: implement support for extended features
In reply to Paolo AbeniQuoted 67 lines · Paolo Abeni
Extend the features configuration space to 128 bits. If the virtio device supports any extended features, allow the common read/write operation to access all of it, otherwise keep exposing only the lower 64 bits.
On migration, save the 128 bit version of the features only if the upper bits are non zero. Relay on reset to clear all the feature space before load.
Signed-off-by Paolo Abeni <[email protected]>---
v3 -> v4:
- use newvirtio_featuresmacro names
- move the build bug beforevmstate_virtio_pci_modern_state_subv2 -> v3:
- drop the pre_load/post_load trickery and relay on reset zeroing the features instead.
- avoid union usage, just increaseguest_featuressize and useSUB_ARRAY.
- drop unneeded '!!'
-_array->_exv1 -> v2:
- use separate VMStateDescription and pre/post load to avoid breaking migration
- clear proxy features on device reset--- hw/virtio/virtio-pci.c | 69 +++++++++++++++++++++++++++++----- include/hw/virtio/virtio-pci.h | 2 +- 2 files changed, 61 insertions(+), 10 deletions(-)hw/virtio/virtio-pci.c +23
-0@@ -109,6 +109,29 @@ static const VMStateDescription vmstate_virtio_pci_modern_queue_state = { } }; +static bool virtio_pci_modern_state_features128_needed (void *opaque) +{ +VirtIOPCIProxy *proxy = opaque; +uint32_t features =0 ; +int i; + +for (i =2 ; i <ARRAY_SIZE (proxy->guest_features); ++i) { + features |= proxy->guest_features[i]; + } +return features; +} + +static const VMStateDescription vmstate_virtio_pci_modern_state_features128 = { + .name ="virtio_pci/modern_state/features128" , + .version_id =1 , + .minimum_version_id =1 , + .needed = &virtio_pci_modern_state_features128_needed, + .fields = (const VMStateField []) { +VMSTATE_UINT32_SUB_ARRAY (guest_features, VirtIOPCIProxy,2 ,2 ), +VMSTATE_END_OF_LIST () + } +}; +static bool virtio_pci_modern_state_needed (void *opaque) {VirtIOPCIProxy *proxy = opaque;
hw/virtio/virtio-pci.c static bool virtio_pci_modern_state_needed(void *opaque) · line 146
Quoted 8 lines
hw/virtio/virtio-pci.c +5
-0@@ -116,6 +139,11 @@ static bool virtio_pci_modern_state_needed(void *opaque)return virtio_pci_modern (proxy); } +/* +* Avoid silently breaking migration should the feature space increase +* even more in the (far away) future +*/ +QEMU_BUILD_BUG_ON (VIRTIO_FEATURES_NU32S !=4 );
Another nitpick: there is a blank line after QEMU_BUILD_BUG_ON() in "[RFC PATCH v4 05/14] virtio: serialize extended features state" but this patch doesn't have a corresponding one. Please keep them consistent.
Quoted 21 lines
hw/virtio/virtio-pci.c +5
-1static const VMStateDescription vmstate_virtio_pci_modern_state_sub = { .name ="virtio_pci/modern_state" , .version_id =1 ,@@ -124,11 +152,15 @@ static const VMStateDescription vmstate_virtio_pci_modern_state_sub = { .fields = (const VMStateField[]) {VMSTATE_UINT32 (dfselect, VirtIOPCIProxy),VMSTATE_UINT32 (gfselect, VirtIOPCIProxy), -VMSTATE_UINT32_ARRAY (guest_features, VirtIOPCIProxy,2 ), +VMSTATE_UINT32_SUB_ARRAY (guest_features, VirtIOPCIProxy,0 ,2 ),VMSTATE_STRUCT_ARRAY (vqs, VirtIOPCIProxy,VIRTIO_QUEUE_MAX ,0 , vmstate_virtio_pci_modern_queue_state, VirtIOPCIQueue),VMSTATE_END_OF_LIST () + }, + .subsections = (const VMStateDescription *const []) { + &vmstate_virtio_pci_modern_state_features128, +NULL } };
hw/virtio/virtio-pci.c int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, · line 1516
Quoted 8 lines
hw/virtio/virtio-pci.c +5
-0@@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,return virtio_pci_add_mem_cap (proxy, &cap.cap); } +static int virtio_pci_select_max (const VirtIODevice *vdev) +{ +return virtio_features_use_ex (vdev->host_features_ex) ? +VIRTIO_FEATURES_NU32S : +2 ;
This function could be simplified by replacing VIRTIO_FEATURES_NU32S without any functional difference:
1. For writes: virtio_set_features_ex() already ignores extended features when !virtio_features_use_ex(vdev->host_features_ex)
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits of host_features_ex are zero, and guest_features upper bits remain zero (since they can't be set per point 1)
So the conditional logic is redundant here.
Quoted 76 lines
hw/virtio/virtio-pci.c +22
-8+} +static uint64_t virtio_pci_common_read (void *opaque,hwaddr addr,unsigned size) {@@ -1494,18 +1533,21 @@ static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr, val = proxy->dfselect;break ;case VIRTIO_PCI_COMMON_DF : -if (proxy->dfselect <=1 ) { +if (proxy->dfselect <virtio_pci_select_max (vdev)) {VirtioDeviceClass *vdc =VIRTIO_DEVICE_GET_CLASS (vdev); - val = (vdev->host_features & ~vdc->legacy_features) >> - (32 * proxy->dfselect); + val = vdev->host_features_ex[proxy->dfselect >>1 ] >> + (32 * (proxy->dfselect &1 )); +if (proxy->dfselect <=1 ) { + val &= (~vdc->legacy_features) >> (32 * proxy->dfselect); + } }break ;case VIRTIO_PCI_COMMON_GFSELECT : val = proxy->gfselect;break ;case VIRTIO_PCI_COMMON_GF : -if (proxy->gfselect <ARRAY_SIZE (proxy->guest_features)) { +if (proxy->gfselect <virtio_pci_select_max (vdev)) { val = proxy->guest_features[proxy->gfselect]; }break ;@@ -1588,11 +1630,18 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr, proxy->gfselect = val;break ;case VIRTIO_PCI_COMMON_GF : -if (proxy->gfselect <ARRAY_SIZE (proxy->guest_features)) { +if (proxy->gfselect <virtio_pci_select_max (vdev)) { +uint64_t features[VIRTIO_FEATURES_NU64S ]; +int i; + proxy->guest_features[proxy->gfselect] = val; -virtio_set_features (vdev, - (((uint64_t )proxy->guest_features[1 ]) <<32 ) | - proxy->guest_features[0 ]); +virtio_features_clear (features); +for (i =0 ; i <ARRAY_SIZE (proxy->guest_features); ++i) { +uint64_t cur = proxy->guest_features[i]; + + features[i >>1 ] |= cur << ((i &1 ) *32 ); + } +virtio_set_features_ex (vdev, features); }break ;case VIRTIO_PCI_COMMON_MSIX :@@ -2311,6 +2360,8 @@ static void virtio_pci_reset(DeviceState *qdev)virtio_bus_reset (bus);msix_unuse_all_vectors (&proxy->pci_dev); +memset (proxy->guest_features,0 ,sizeof (proxy->guest_features)); +for (i =0 ; i <VIRTIO_QUEUE_MAX ; i++) { proxy->vqs[i].enabled =0 ; proxy->vqs[i].reset =0 ;include/hw/virtio/virtio-pci.h +1
-1@@ -158,7 +158,7 @@ struct VirtIOPCIProxy {uint32_t nvectors;uint32_t dfselect;uint32_t gfselect; -uint32_t guest_features[2 ]; +uint32_t guest_features[VIRTIO_FEATURES_NU32S ];VirtIOPCIQueue vqs[VIRTIO_QUEUE_MAX ];VirtIOIRQFD *vector_irqfd;
Paolo Abeni
maintainerRe: [RFC PATCH v4 07/14] virtio-pci: implement support for extended features
Quoted 14 lines · Akihiko Odaki
hw/virtio/virtio-pci.c int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, · line 1516
Quoted 9 lines · Paolo Abeni
hw/virtio/virtio-pci.c +5
-0@@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,return virtio_pci_add_mem_cap (proxy, &cap.cap); } +static int virtio_pci_select_max (const VirtIODevice *vdev) +{ +return virtio_features_use_ex (vdev->host_features_ex) ? +VIRTIO_FEATURES_NU32S : +2 ;This function could be simplified by replacing
VIRTIO_FEATURES_NU32Swithout any functional difference:
Quoted 7 lines
1. For writes:
virtio_set_features_ex()already ignores extended features when !virtio_features_use_ex(vdev->host_features_ex)
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits ofhost_features_exare zero, andguest_featuresupper bits remain zero (since they can't be set per point 1)So the conditional logic is redundant here.
This is to satisfy a request from Jason:
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05291.html
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05423.html
I agree there will not be functional differences always accessing the full space, but the guest could still be able to notice, i.e. the extended space will be zeroed on read with that patched qemu and untouched by the current code and this patch. To be on the safe side I think it would be better to avoid such difference, as suggested by Jason.
Does the above make sense to you?
Thanks,
Paolo
Re: [RFC PATCH v4 07/14] virtio-pci: implement support for extended features
Quoted 28 lines · Paolo Abeni
Quoted 14 lines · Akihiko Odaki
hw/virtio/virtio-pci.c int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, · line 1516
Quoted 9 lines · Paolo Abeni
hw/virtio/virtio-pci.c +5
-0@@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,return virtio_pci_add_mem_cap (proxy, &cap.cap); } +static int virtio_pci_select_max (const VirtIODevice *vdev) +{ +return virtio_features_use_ex (vdev->host_features_ex) ? +VIRTIO_FEATURES_NU32S : +2 ;This function could be simplified by replacing
VIRTIO_FEATURES_NU32Swithout any functional difference:Quoted 7 lines
1. For writes:
virtio_set_features_ex()already ignores extended features when !virtio_features_use_ex(vdev->host_features_ex)
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits ofhost_features_exare zero, andguest_featuresupper bits remain zero (since they can't be set per point 1)So the conditional logic is redundant here.
This is to satisfy a request from Jason:
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05291.html
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05423.html
Quoted 7 lines
I agree there will not be functional differences always accessing the full space, but the guest could still be able to notice, i.e. the extended space will be zeroed on read with that patched qemu and untouched by the current code and this patch. To be on the safe side I think it would be better to avoid such difference, as suggested by Jason.
Does the above make sense to you?
By functional, I meant the functionality of QEMU, visible to the guest, rather than the whole system including the guest, visible to the end user. The guest cannot notice the difference because the extended space is zero on read even without the conditional, which is described as point 2 in the previous email.
Quoted 3 lines
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits of
host_features_exare zero, andguest_featuresupper bits remain zero (since they can't be set per point 1)
Regards,
Akihiko Odaki
Re: [RFC PATCH v4 07/14] virtio-pci: implement support for extended features
Quoted 8 lines · Akihiko Odaki
Quoted 6 lines · Paolo Abeni
Quoted 5 lines · Akihiko Odaki
Quoted 4 lines · Paolo Abeni
hw/virtio/virtio-pci.c +0
-0@@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,return virtio_pci_add_mem_cap (proxy, &cap.cap); }
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
hw/virtio/virtio-pci.c int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, · line 1516
Quoted 5 lines
hw/virtio/virtio-pci.c +5
-0+static int virtio_pci_select_max (const VirtIODevice *vdev) +{ +return virtio_features_use_ex (vdev->host_features_ex) ? +VIRTIO_FEATURES_NU32S : +2 ;This function could be simplified by replacing
VIRTIO_FEATURES_NU32Swithout any functional difference:
Did you mean using VIRTIO_FEATURES_NU32S instead?
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
1. For writes:
virtio_set_features_ex()already ignores extended features when !virtio_features_use_ex(vdev->host_features_ex)
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits ofhost_features_exare zero, andguest_featuresupper bits remain zero (since they can't be set per point 1)
I think it depends on the compatibility work which hasn't been done in this series.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
So the conditional logic is redundant here.
See below
Quoted 12 lines
Quoted 12 lines
This is to satisfy a request from Jason:
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05291.html
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05423.htmlI agree there will not be functional differences always accessing the full space, but the guest could still be able to notice, i.e. the extended space will be zeroed on read with that patched qemu and untouched by the current code and this patch. To be on the safe side I think it would be better to avoid such difference, as suggested by Jason.
Quoted 7 lines
Quoted 1 line
Does the above make sense to you?
By functional, I meant the functionality of QEMU, visible to the guest, rather than the whole system including the guest, visible to the end user. The guest cannot notice the difference because the extended space is zero on read even without the conditional, which is described as point 2 in the previous email.
I'm not sure I understand this correctly. But it doesn't harm here consider:
1) it's the entry point from the guest, checking and failing early is better than depending on the low layer functions
2) we have checks in several layers (both virtio-pci and virtio core).
And it looks like a must for at least GF:
case VIRTIO_PCI_COMMON_GF:
if (proxy->gfselect < virtio_pci_select_max(vdev)) {
uint64_t features[VIRTIO_FEATURES_NU64S];
int i;
proxy->guest_features[proxy->gfselect] = val;Thanks
Quoted 8 lines
Quoted 4 lines
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits of
host_features_exare zero, andguest_featuresupper bits remain zero (since they can't be set per point 1)Regards,
Akihiko Odaki
Re: [RFC PATCH v4 07/14] virtio-pci: implement support for extended features
Quoted 62 lines · Jason Wang
Quoted 8 lines · Akihiko Odaki
Quoted 6 lines · Paolo Abeni
Quoted 5 lines · Akihiko Odaki
Quoted 4 lines · Paolo Abeni
hw/virtio/virtio-pci.c +0
-0@@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,return virtio_pci_add_mem_cap (proxy, &cap.cap); }Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
hw/virtio/virtio-pci.c int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, · line 1516
Quoted 5 lines
hw/virtio/virtio-pci.c +5
-0+static int virtio_pci_select_max (const VirtIODevice *vdev) +{ +return virtio_features_use_ex (vdev->host_features_ex) ? +VIRTIO_FEATURES_NU32S : +2 ;This function could be simplified by replacing
VIRTIO_FEATURES_NU32Swithout any functional difference:Did you mean using
VIRTIO_FEATURES_NU32Sinstead?Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
1. For writes:
virtio_set_features_ex()already ignores extended features when !virtio_features_use_ex(vdev->host_features_ex)
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits ofhost_features_exare zero, andguest_featuresupper bits remain zero (since they can't be set per point 1)I think it depends on the compatibility work which hasn't been done in this series.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
So the conditional logic is redundant here.
See below
Quoted 12 lines
Quoted 12 lines
This is to satisfy a request from Jason:
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05291.html
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05423.htmlI agree there will not be functional differences always accessing the full space, but the guest could still be able to notice, i.e. the extended space will be zeroed on read with that patched qemu and untouched by the current code and this patch. To be on the safe side I think it would be better to avoid such difference, as suggested by Jason.
Quoted 7 lines
Quoted 1 line
Does the above make sense to you?
By functional, I meant the functionality of QEMU, visible to the guest, rather than the whole system including the guest, visible to the end user. The guest cannot notice the difference because the extended space is zero on read even without the conditional, which is described as point 2 in the previous email.
I'm not sure I understand this correctly. But it doesn't harm here consider:
1) it's the entry point from the guest, checking and failing early is better than depending on the low layer functions
2) we have checks in several layers (both virtio-pci and virtio core).
Quoted 8 lines
And it looks like a must for at least GF:
case VIRTIO_PCI_COMMON_GF: if (proxy->gfselect < virtio_pci_select_max(vdev)) { uint64_t features[VIRTIO_FEATURES_NU64S]; int i; proxy->guest_features[proxy->gfselect] = val;
I missed proxy->guest_features. Indeed it makes a difference.
Now I have another concern with virtio_pci_select_max(). If the feature set grows again, virtio_pci_select_max() will return the grown size for the devices that have [127:64] bits, which will be a breaking change. In this sense, VIRTIO_FEATURES_NU32S, which will grow along with the feature set, is not appropriate here.
Hardcoding 4 ensures such a breaking change will not happen at least.
Perhaps QEMU_BUILD_BUG_ON() may be placed here to tell it needs to be updated when the feature set grows, but I don't require that since there is QEMU_BUILD_BUG_ON() before vmstate_virtio_pci_modern_state_sub. It may be a good idea to move this function somewhere close to QEMU_BUILD_BUG_ON().
Alternatively the function may compute the value to return by finding the last non-zero element in vdev->host_features_ex and multiplying by 2. It will probably work even if the feature set grows again in the future.
Regards,
Akihiko Odaki
Re: [RFC PATCH v4 07/14] virtio-pci: implement support for extended features
Quoted 73 lines · Akihiko Odaki
Quoted 62 lines · Jason Wang
Quoted 8 lines · Akihiko Odaki
Quoted 6 lines · Paolo Abeni
Quoted 5 lines · Akihiko Odaki
Quoted 4 lines · Paolo Abeni
hw/virtio/virtio-pci.c +0
-0@@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,return virtio_pci_add_mem_cap (proxy, &cap.cap); }Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
hw/virtio/virtio-pci.c int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, · line 1516
Quoted 5 lines
hw/virtio/virtio-pci.c +5
-0+static int virtio_pci_select_max (const VirtIODevice *vdev) +{ +return virtio_features_use_ex (vdev->host_features_ex) ? +VIRTIO_FEATURES_NU32S : +2 ;This function could be simplified by replacing
VIRTIO_FEATURES_NU32Swithout any functional difference:Did you mean using
VIRTIO_FEATURES_NU32Sinstead?Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
1. For writes:
virtio_set_features_ex()already ignores extended features when !virtio_features_use_ex(vdev->host_features_ex)
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits ofhost_features_exare zero, andguest_featuresupper bits remain zero (since they can't be set per point 1)I think it depends on the compatibility work which hasn't been done in this series.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
So the conditional logic is redundant here.
See below
Quoted 12 lines
Quoted 12 lines
This is to satisfy a request from Jason:
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05291.html
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05423.htmlI agree there will not be functional differences always accessing the full space, but the guest could still be able to notice, i.e. the extended space will be zeroed on read with that patched qemu and untouched by the current code and this patch. To be on the safe side I think it would be better to avoid such difference, as suggested by Jason.
Quoted 7 lines
Quoted 1 line
Does the above make sense to you?
By functional, I meant the functionality of QEMU, visible to the guest, rather than the whole system including the guest, visible to the end user. The guest cannot notice the difference because the extended space is zero on read even without the conditional, which is described as point 2 in the previous email.
I'm not sure I understand this correctly. But it doesn't harm here consider:
1) it's the entry point from the guest, checking and failing early is better than depending on the low layer functions
2) we have checks in several layers (both virtio-pci and virtio core).Quoted 8 lines
And it looks like a must for at least GF:
case VIRTIO_PCI_COMMON_GF: if (proxy->gfselect < virtio_pci_select_max(vdev)) { uint64_t features[VIRTIO_FEATURES_NU64S]; int i; proxy->guest_features[proxy->gfselect] = val;
Quoted 7 lines
I missed
proxy->guest_features. Indeed it makes a difference.Now I have another concern with
virtio_pci_select_max(). If the feature set grows again,virtio_pci_select_max()will return the grown size for the devices that have [127:64] bits, which will be a breaking change. In this sense,VIRTIO_FEATURES_NU32S, which will grow along with the feature set, is not appropriate here.
Things will be fine if we do the compatibility work correctly, so host "host_features_ex_ex" won't contain anything for the legacy machine types.
But I'm not sure it's worth worrying about it now considering we take years to reach 64.
Quoted 9 lines
Hardcoding 4 ensures such a breaking change will not happen at least.
Perhaps
QEMU_BUILD_BUG_ON()may be placed here to tell it needs to be updated when the feature set grows, but I don't require that since there isQEMU_BUILD_BUG_ON()beforevmstate_virtio_pci_modern_state_sub. It may be a good idea to move this function somewhere close toQEMU_BUILD_BUG_ON().
Quoted 6 lines
Alternatively the function may compute the value to return by finding the last non-zero element in
vdev->host_features_exand multiplying by
2. It will probably work even if the feature set grows again in the future.Regards,
Akihiko Odaki
Thanks
Quoted 1 line
Paolo Abeni
maintainerRe: [RFC PATCH v4 07/14] virtio-pci: implement support for extended features
Quoted 83 lines · Jason Wang
Quoted 73 lines · Akihiko Odaki
Quoted 62 lines · Jason Wang
Quoted 8 lines · Akihiko Odaki
Quoted 6 lines · Paolo Abeni
Quoted 5 lines · Akihiko Odaki
Quoted 4 lines · Paolo Abeni
(quoted diff) +0
-0@@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, return virtio_pci_add_mem_cap(proxy, &cap.cap); }Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Review int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, · line 1516
Quoted 5 lines
(quoted diff) +5
-0+static int virtio_pci_select_max(const VirtIODevice *vdev) +{ + return virtio_features_use_ex(vdev->host_features_ex) ? + VIRTIO_FEATURES_NU32S : + 2;This function could be simplified by replacing
VIRTIO_FEATURES_NU32Swithout any functional difference:Did you mean using
VIRTIO_FEATURES_NU32Sinstead?Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
1. For writes:
virtio_set_features_ex()already ignores extended features when !virtio_features_use_ex(vdev->host_features_ex)
2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the upper bits ofhost_features_exare zero, andguest_featuresupper bits remain zero (since they can't be set per point 1)I think it depends on the compatibility work which hasn't been done in this series.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
So the conditional logic is redundant here.
See below
Quoted 12 lines
Quoted 12 lines
This is to satisfy a request from Jason:
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05291.html
https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05423.htmlI agree there will not be functional differences always accessing the full space, but the guest could still be able to notice, i.e. the extended space will be zeroed on read with that patched qemu and untouched by the current code and this patch. To be on the safe side I think it would be better to avoid such difference, as suggested by Jason.
Quoted 7 lines
Quoted 1 line
Does the above make sense to you?
By functional, I meant the functionality of QEMU, visible to the guest, rather than the whole system including the guest, visible to the end user. The guest cannot notice the difference because the extended space is zero on read even without the conditional, which is described as point 2 in the previous email.
I'm not sure I understand this correctly. But it doesn't harm here consider:
1) it's the entry point from the guest, checking and failing early is better than depending on the low layer functions
2) we have checks in several layers (both virtio-pci and virtio core).Quoted 8 lines
And it looks like a must for at least GF:
case VIRTIO_PCI_COMMON_GF: if (proxy->gfselect < virtio_pci_select_max(vdev)) { uint64_t features[VIRTIO_FEATURES_NU64S]; int i; proxy->guest_features[proxy->gfselect] = val;Quoted 7 lines
I missed
proxy->guest_features. Indeed it makes a difference.Now I have another concern with
virtio_pci_select_max(). If the feature set grows again,virtio_pci_select_max()will return the grown size for the devices that have [127:64] bits, which will be a breaking change. In this sense,VIRTIO_FEATURES_NU32S, which will grow along with the feature set, is not appropriate here.
Quoted 6 lines
Things will be fine if we do the compatibility work correctly, so host "
host_features_ex_ex" won't contain anything for the legacy machine types.But I'm not sure it's worth worrying about it now considering we take years to reach 64.
FTR, I gave a shot to the last option mentioned by Akihiko - make virtio_pci_select_max() future proof - and the implementation complexity is almost the same of the current one, so I guess I'll opt for such a thing.
Cheers,
Paolo