Virtualization - Aug 2022 - [PATCH 2/2] vDPA: conditionally read fields in virtio-net dev

? 2022/8/17 18:37, Michael S. Tsirkin ??:
> On Wed, Aug 17, 2022 at 05:43:22PM +0800, Zhu, Lingshan wrote:
>>
>> On 8/17/2022 5:39 PM, Michael S. Tsirkin wrote:
>>> On Wed, Aug 17, 2022 at 05:13:59PM +0800, Zhu, Lingshan wrote:
>>>> On 8/17/2022 4:55 PM, Michael S. Tsirkin wrote:
>>>>> On Wed, Aug 17, 2022 at 10:14:26AM +0800, Zhu, Lingshan
wrote:
>>>>>> Yes it is a little messy, and we can not check
_F_VERSION_1 because of
>>>>>> transitional devices, so maybe this is the best we can
do for now
>>>>> I think vhost generally needs an API to declare config
space endian-ness
>>>>> to kernel. vdpa can reuse that too then.
>>>> Yes, I remember you have mentioned some IOCTL to set the
endian-ness,
>>>> for vDPA, I think only the vendor driver knows the endian,
>>>> so we may need a new function vdpa_ops->get_endian().
>>>> In the last thread, we say maybe it's better to add a
comment for now.
>>>> But if you think we should add a vdpa_ops->get_endian(), I
can work
>>>> on it for sure!
>>>>
>>>> Thanks
>>>> Zhu Lingshan
>>> I think QEMU has to set endian-ness. No one else knows.
>> Yes, for SW based vhost it is true. But for HW vDPA, only
>> the device & driver knows the endian, I think we can not
>> "set" a hardware's endian.
> QEMU knows the guest endian-ness and it knows that
> device is accessed through the legacy interface.
> It can accordingly send endian-ness to the kernel and
> kernel can propagate it to the driver.

I wonder if we can simply force LE and then Qemu can do the endian 
conversion?

Thanks

>
>> So if you think we should add a vdpa_ops->get_endian(),
>> I will drop these comments in the next version of
>> series, and work on a new patch for get_endian().
>>
>> Thanks,
>> Zhu Lingshan
> Guests don't get endian-ness from devices so this seems pointless.
>

On 8/17/2022 9:15 PM, Jason Wang wrote:
>
> ? 2022/8/17 18:37, Michael S. Tsirkin ??:
>> On Wed, Aug 17, 2022 at 05:43:22PM +0800, Zhu, Lingshan wrote:
>>>
>>> On 8/17/2022 5:39 PM, Michael S. Tsirkin wrote:
>>>> On Wed, Aug 17, 2022 at 05:13:59PM +0800, Zhu, Lingshan wrote:
>>>>> On 8/17/2022 4:55 PM, Michael S. Tsirkin wrote:
>>>>>> On Wed, Aug 17, 2022 at 10:14:26AM +0800, Zhu, Lingshan
wrote:
>>>>>>> Yes it is a little messy, and we can not check
_F_VERSION_1
>>>>>>> because of
>>>>>>> transitional devices, so maybe this is the best we
can do for now
>>>>>> I think vhost generally needs an API to declare config
space
>>>>>> endian-ness
>>>>>> to kernel. vdpa can reuse that too then.
>>>>> Yes, I remember you have mentioned some IOCTL to set the
endian-ness,
>>>>> for vDPA, I think only the vendor driver knows the endian,
>>>>> so we may need a new function vdpa_ops->get_endian().
>>>>> In the last thread, we say maybe it's better to add a
comment for
>>>>> now.
>>>>> But if you think we should add a vdpa_ops->get_endian(),
I can work
>>>>> on it for sure!
>>>>>
>>>>> Thanks
>>>>> Zhu Lingshan
>>>> I think QEMU has to set endian-ness. No one else knows.
>>> Yes, for SW based vhost it is true. But for HW vDPA, only
>>> the device & driver knows the endian, I think we can not
>>> "set" a hardware's endian.
>> QEMU knows the guest endian-ness and it knows that
>> device is accessed through the legacy interface.
>> It can accordingly send endian-ness to the kernel and
>> kernel can propagate it to the driver.
>
>
> I wonder if we can simply force LE and then Qemu can do the endian 
> conversion?
convert from LE for config space fields only, or QEMU has to forcefully 
mediate and covert endianness for all device memory access including 
even the datapath (fields in descriptor and avail/used rings)? I hope 
it's not the latter, otherwise it loses the point to use vDPA for 
datapath acceleration.

Even if its the former, it's a little weird for vendor device to 
implement a LE config space with BE ring layout, although still possible...

-Siwei
>
> Thanks
>
>
>>
>>> So if you think we should add a vdpa_ops->get_endian(),
>>> I will drop these comments in the next version of
>>> series, and work on a new patch for get_endian().
>>>
>>> Thanks,
>>> Zhu Lingshan
>> Guests don't get endian-ness from devices so this seems pointless.
>>
>