Re: [PATCH net-next 1/8] virtio: introduce virtio_features_t

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, May 26, 2025 at 09:20:50AM +0200, Paolo Abeni wrote:
> On 5/26/25 2:43 AM, Jason Wang wrote:
> > On Wed, May 21, 2025 at 6:33 PM Paolo Abeni <pabeni@redhat.com> wrote:
> >> diff --git a/include/linux/virtio_features.h b/include/linux/virtio_features.h
> >> new file mode 100644
> >> index 0000000000000..2f742eeb45a29
> >> --- /dev/null
> >> +++ b/include/linux/virtio_features.h
> >> @@ -0,0 +1,23 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +#ifndef _LINUX_VIRTIO_FEATURES_H
> >> +#define _LINUX_VIRTIO_FEATURES_H
> >> +
> >> +#include <linux/bits.h>
> >> +
> >> +#if IS_ENABLED(CONFIG_ARCH_SUPPORTS_INT128)
> >> +#define VIRTIO_HAS_EXTENDED_FEATURES
> >> +#define VIRTIO_FEATURES_MAX    128
> >> +#define VIRTIO_FEATURES_WORDS  4
> >> +#define VIRTIO_BIT(b)          _BIT128(b)
> >> +
> >> +typedef __uint128_t            virtio_features_t;
> > 
> > Consider:
> > 
> > 1) need the trick for arch that doesn't support 128bit
> > 2) some transport (e.g PCI) allows much more than just 128 bit features
> > 
> >  I wonder if it's better to just use arrays here.
> 
> I considered that, it has been discussed both on the virtio ML and
> privatelly, and I tried a resonable attempt with such implementation.
> 
> The diffstat would be horrible, touching a lot of the virtio/vhost code.
> Such approach will block any progress for a long time (more likely
> forever, since I will not have the capacity to complete it).
> 
> Also the benefit are AFAICS marginal, as 32 bits platform with huge
> virtualization deployments on top of it (that could benefit from GSO
> over UDP tunnel) are IMHO unlikely, and transport features space
> exhaustion is AFAIK far from being reached (also thanks to reserved
> features availables).
> 
> TL;DR: if you consider a generic implementation for an arbitrary wide
> features space blocking, please LMK, because any other consideration
> would be likely irrelevant otherwise.
> 
> /P

Let's just say, I'm fine with starting with this, and
we can move to an array later. The nice thing here
is that there's this typedef, it can later be changed to be
any struct at all.