Virtualization - Dec 2021 - vdpa legacy guest support (was Re: [PATCH] vdpa/mlx5: set_features should allow reset to zero)

On 2021/3/2 5:47 ??, Michael S. Tsirkin wrote:
> On Mon, Mar 01, 2021 at 11:56:50AM +0800, Jason Wang wrote:
>> On 2021/3/1 5:34 ??, Michael S. Tsirkin wrote:
>>> On Wed, Feb 24, 2021 at 10:24:41AM -0800, Si-Wei Liu wrote:
>>>>> Detecting it isn't enough though, we will need a new
ioctl to notify
>>>>> the kernel that it's a legacy guest. Ugh :(
>>>> Well, although I think adding an ioctl is doable, may I know
what the use
>>>> case there will be for kernel to leverage such info directly?
Is there a
>>>> case QEMU can't do with dedicate ioctls later if
there's indeed
>>>> differentiation (legacy v.s. modern) needed?
>>> BTW a good API could be
>>>
>>> #define VHOST_SET_ENDIAN _IOW(VHOST_VIRTIO, ?, int)
>>> #define VHOST_GET_ENDIAN _IOW(VHOST_VIRTIO, ?, int)
>>>
>>> we did it per vring but maybe that was a mistake ...
>>
>> Actually, I wonder whether it's good time to just not support
legacy driver
>> for vDPA. Consider:
>>
>> 1) It's definition is no-normative
>> 2) A lot of budren of codes
>>
>> So qemu can still present the legacy device since the config space or
other
>> stuffs that is presented by vhost-vDPA is not expected to be accessed
by
>> guest directly. Qemu can do the endian conversion when necessary in
this
>> case?
>>
>> Thanks
>>
> Overall I would be fine with this approach but we need to avoid breaking
> working userspace, qemu releases with vdpa support are out there and
> seem to work for people. Any changes need to take that into account
> and document compatibility concerns.

Agree, let me check.

>   I note that any hardware
> implementation is already broken for legacy except on platforms with
> strong ordering which might be helpful in reducing the scope.

Yes.

Thanks

>
>

Sorry for reviving this ancient thread. I was kinda lost for the 
conclusion it ended up with. I have the following questions,

1. legacy guest support: from the past conversations it doesn't seem the 
support will be completely dropped from the table, is my understanding 
correct? Actually we're interested in supporting virtio v0.95 guest for 
x86, which is backed by the spec at 
https://ozlabs.org/~rusty/virtio-spec/virtio-0.9.5.pdf. Though I'm not 
sure if there's request/need to support wilder legacy virtio versions 
earlier beyond.

2. suppose some form of legacy guest support needs to be there, how do 
we deal with the bogus assumption below in vdpa_get_config() in the 
short term? It looks one of the intuitive fix is to move the 
vdpa_set_features call out of vdpa_get_config() to vdpa_set_config().

 ??????? /*
 ???????? * Config accesses aren't supposed to trigger before features 
are set.
 ???????? * If it does happen we assume a legacy guest.
 ???????? */
 ??????? if (!vdev->features_valid)
 ??????????????? vdpa_set_features(vdev, 0);
 ??????? ops->get_config(vdev, offset, buf, len);

I can post a patch to fix 2) if there's consensus already reached.

Thanks,
-Siwei

On 3/2/2021 2:53 AM, Jason Wang wrote:
>
> On 2021/3/2 5:47 ??, Michael S. Tsirkin wrote:
>> On Mon, Mar 01, 2021 at 11:56:50AM +0800, Jason Wang wrote:
>>> On 2021/3/1 5:34 ??, Michael S. Tsirkin wrote:
>>>> On Wed, Feb 24, 2021 at 10:24:41AM -0800, Si-Wei Liu wrote:
>>>>>> Detecting it isn't enough though, we will need a
new ioctl to notify
>>>>>> the kernel that it's a legacy guest. Ugh :(
>>>>> Well, although I think adding an ioctl is doable, may I
know what
>>>>> the use
>>>>> case there will be for kernel to leverage such info
directly? Is
>>>>> there a
>>>>> case QEMU can't do with dedicate ioctls later if
there's indeed
>>>>> differentiation (legacy v.s. modern) needed?
>>>> BTW a good API could be
>>>>
>>>> #define VHOST_SET_ENDIAN _IOW(VHOST_VIRTIO, ?, int)
>>>> #define VHOST_GET_ENDIAN _IOW(VHOST_VIRTIO, ?, int)
>>>>
>>>> we did it per vring but maybe that was a mistake ...
>>>
>>> Actually, I wonder whether it's good time to just not support
legacy
>>> driver
>>> for vDPA. Consider:
>>>
>>> 1) It's definition is no-normative
>>> 2) A lot of budren of codes
>>>
>>> So qemu can still present the legacy device since the config space 
>>> or other
>>> stuffs that is presented by vhost-vDPA is not expected to be 
>>> accessed by
>>> guest directly. Qemu can do the endian conversion when necessary in
>>> this
>>> case?
>>>
>>> Thanks
>>>
>> Overall I would be fine with this approach but we need to avoid
breaking
>> working userspace, qemu releases with vdpa support are out there and
>> seem to work for people. Any changes need to take that into account
>> and document compatibility concerns.
>
>
> Agree, let me check.
>
>
>> ? I note that any hardware
>> implementation is already broken for legacy except on platforms with
>> strong ordering which might be helpful in reducing the scope.
>
>
> Yes.
>
> Thanks
>
>
>>
>>
>