Re: [PATCH] vdpa: Allow vDPA to work on big-endian machine

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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 <kshk@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...
>>
>> 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.