On Tue, Feb 18, 2025 at 10:03 PM Philippe Mathieu-Daudé
<phi...@linaro.org> wrote:
>
> Hi Konstantin,
>
> (Cc'ing more developers)
>
> On 18/2/25 14:27, Konstantin Shkolnyy wrote:
> > On 2/12/2025 14:01, Konstantin Shkolnyy wrote:
> >> On 2/12/2025 12:07, Philippe Mathieu-Daudé wrote:
> >>> On 12/2/25 18:24, Konstantin Shkolnyy wrote:
> >>>> On 2/12/2025 08:52, Philippe Mathieu-Daudé wrote:
> >>>>> On 11/2/25 17:19, Konstantin Shkolnyy wrote:
> >>>>>> Add .set_vnet_le() function that always returns success, assuming
> >>>>>> that
> >>>>>> vDPA h/w always implements LE data format. Otherwise, QEMU
> >>>>>> disables vDPA and
> >>>>>> outputs the message:
> >>>>>> "backend does not support LE vnet headers; falling back on
> >>>>>> userspace virtio"
> >>>>>>
> >>>>>> Signed-off-by: Konstantin Shkolnyy <k...@linux.ibm.com>
> >>>>>> ---
> >>>>>> net/vhost-vdpa.c | 6 ++++++
> >>>>>> 1 file changed, 6 insertions(+)
> >>>>>>
> >>>>>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> >>>>>> index 231b45246c..7219aa2eee 100644
> >>>>>> --- a/net/vhost-vdpa.c
> >>>>>> +++ b/net/vhost-vdpa.c
> >>>>>> @@ -270,6 +270,11 @@ static bool vhost_vdpa_has_ufo(NetClientState
> >>>>>> *nc)
> >>>>>> }
> >>>>>> +static int vhost_vdpa_set_vnet_le(NetClientState *nc, bool is_le)
> >>>>>> +{
> >>>>>> + return 0;
> >>>>>> +}
> >>>>>> +
> >>>>>> static bool vhost_vdpa_check_peer_type(NetClientState *nc,
> >>>>>> ObjectClass *oc,
> >>>>>> Error **errp)
> >>>>>> {
> >>>>>> @@ -437,6 +442,7 @@ static NetClientInfo net_vhost_vdpa_info = {
> >>>>>> .cleanup = vhost_vdpa_cleanup,
> >>>>>> .has_vnet_hdr = vhost_vdpa_has_vnet_hdr,
> >>>>>> .has_ufo = vhost_vdpa_has_ufo,
> >>>>>> + .set_vnet_le = vhost_vdpa_set_vnet_le,
> >>>>>
> >>>>> Dubious mismatch with set_vnet_be handler.
> >>>>
> >>>> I'm not sure what you are suggesting...
> >>>
> >>> Implement set_vnet_le for parity?
> >>
> >> To my (very limited) knowledge, kernel's vhost_vdpa that QEMU talks to
> >> doesn't have an API to "change h/w endianness". If so,
> >> vDPA's .set_vnet_le/be(), as well as qemu_set_vnet_le/be() have very
> >> limited choices. qemu_set_vnet_le/be() behavior with vDPA was to
> >> simply assume that h/w endianness by default matches host's. This
> >> assumption is valid for other types of "NetClients" which are
> >> implemented in s/w. However, I suspect, vDPA h/w might all be going to
> >> be LE, to match virtio 1.0. Such is the NIC I'm dealing with.
> >>
> >> My patch is only fixing a specific use case. Perhaps, for a complete
> >> fix, qemu_set_vnet_be() also shouldn't unconditionally return success
> >> on big endian machines, but always call .set_vnet_be() so that vDPA
> >> could fail it? But then it would start calling .set_vnet_be() on other
> >> "NetClients" where it didn't before.
> >>
> >> That's why I don't want to just add a .set_vnet_be(), before someone
> >> here even confirms that vDPA h/w is indeed assumed LE, and, hence,
> >> what the right path is to a complete solution...
Note that modern (VERION_1) virtio/vDPA device assumes LE.
Transitional devices need to support "native endian" which requires
the interface like what you suggest here when guest and host endian
are different.
But we need to confirm with the vDPA vendors to know if their device
is modern only or transitional.
E.g by looking at mlx5_vdpa drivers, it seems to support big endian:
/* TODO: cross-endian support */
static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
{
return virtio_legacy_is_little_endian() ||
(mvdev->actual_features & BIT_ULL(VIRTIO_F_VERSION_1));
}
But it needs to judge the endian according to what userspace tells us
(as the "TODO"), we probably need to invent new vDPA ioctls for that
besides the Qemu modifications.
Adding Dragos for more thought here.
Thanks
> >>
> >> int qemu_set_vnet_be(NetClientState *nc, bool is_be)
> >> {
> >> #if HOST_BIG_ENDIAN
> >> return 0;
> >> #else
> >> if (!nc || !nc->info->set_vnet_be)
> >> return -ENOSYS;
> >>
> >> return nc->info->set_vnet_be(nc, is_be);
> >> #endif
> >> }
> >>
> >
> > Does anyone have any answers/suggestions?
> >
>
> Since you mentioned "vDPA h/w always implements LE data format",
> I'd expect virtio_is_big_endian(vdev) always return FALSE, and
> thus this to be safe:
>
> static int vhost_vdpa_set_vnet_be(NetClientState *nc, bool is_le)
> {
> g_assert_not_reached();
> }
>
> But I don't know much about vDPA, so I won't object to your patch.
>
> Regards,
>
> Phil.
>
