Re: [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification

[Jason Wang <jasowang@redhat.com>]

在 2021/8/2 下午11:27, Max Gurtovoy 写道:
>
> On 8/2/2021 5:51 PM, Cornelia Huck wrote:
>> On Mon, Aug 02 2021, Max Gurtovoy <mgurtovoy@nvidia.com> wrote:
>>
>>> On 8/2/2021 5:17 AM, Jason Wang wrote:
>>>> 在 2021/8/1 上午6:53, Max Gurtovoy 写道:
>>>>> On 8/1/2021 1:26 AM, Michael S. Tsirkin wrote:
>>>>>> On Sat, Jul 31, 2021 at 02:34:25PM +0300, Max Gurtovoy wrote:
>>>>>>> On 7/30/2021 10:05 AM, Cornelia Huck wrote:
>>>>>>>> On Thu, Jul 29 2021, Max Gurtovoy <mgurtovoy@nvidia.com> wrote:
>>>>>>>>
>>>>>>>>> On 7/28/2021 3:48 PM, Michael S. Tsirkin wrote:
>>>>>>>>>> On Mon, Jul 26, 2021 at 07:52:53PM +0300, Max Gurtovoy wrote:
>>>>>>>>>>> +\subsubsection{Vendor specific command set}\label{sec:Basic
>>>>>>>>>>> Facilities of a Virtio Device / Admin Virtqueues / Admin
>>>>>>>>>>> command set / Vendor specific command set}
>>>>>>>>>>> +
>>>>>>>>>>> +The Vendor specific command set is a group of classes and
>>>>>>>>>>> commands
>>>>>>>>>>> +within each of these classes which are vendor specific. 
>>>>>>>>>>> Refer to
>>>>>>>>>>> +each vendor specification for more information on the 
>>>>>>>>>>> supported
>>>>>>>>>>> +commands.
>>>>>>>>>> Here's another example.
>>>>>>>>>> It's important that there is a way to make a device completely
>>>>>>>>>> generic without vendor specific expensions.
>>>>>>>>>> Would be hard to do here.
>>>>>>>>>>
>>>>>>>>>> That's a generic comment.
>>>>>>>>>>
>>>>>>>>>> but specifically I am very reluctant to add vendor specific
>>>>>>>>>> stuff like
>>>>>>>>>> this directly in the spec. We already have
>>>>>>>>>> VIRTIO_PCI_CAP_VENDOR_CFG
>>>>>>>>>> and if that is not sufficient I would like to know why
>>>>>>>>>> before we add more vendor specific stuff.
>>>>>>>>> We are adding an option to add vendor specific commands. We're 
>>>>>>>>> not
>>>>>>>>> adding it to the spec since each vendor will have its own 
>>>>>>>>> manual for
>>>>>>>>> that.
>>>>>>>> IMHO, that way madness lies. I want to be able to look at the spec
>>>>>>>> and
>>>>>>>> be able to implement a compliant device or a compliant driver. 
>>>>>>>> If a
>>>>>>>> vendor has some special feature they want to support, put it in 
>>>>>>>> the
>>>>>>>> spec, so that it is possible to actually implement it.
>>>>>>> You can implement a compliant device and a compliant. why do you
>>>>>>> think you
>>>>>>> can't ?
>> If I want to implement a device/driver, I need a standard to refer
>> to. How can something vendor-specific be standard? If it is useful, add
>> it to the virtio spec.
>
> again, Linux driver will be generic.
>
> It will include a char dev to user space virtio-cli that will enable 
> sending admin commands to do administration.
>
> Nobody asked to add vendor specific code to the driver.


It's not about the general driver but about the general device. We want 
to present a general standard device which means we can migrate virtio 
devices among different vendors.

If each vendor choose to implement their own vendor specific features 
via vendor specific commands that are not ruled by the spec, there will 
be little value of the spec.


>
> The spec allows vendor specific stuff today that are not usable and 
> I'm trying to create a usable API without adding one line of vendor 
> code to driver.
>
> You just need to open a window for few commands like others do and 
> like you already did in the spec.
>
> NVIDIA HW internals are different from VENDOR_A and VENDOR_B.
>
> I want to debug my HW or enable some accelerations in it.


A big question is that, on what layer do you want to provide those 
functions?

1) virtio level
2) vendor level

For 1), it needs to be done in the spec instead of introducing backdoor 
for vendor specific command

For 2), virtio keeps simple as is, functionality is provided in a vendor 
specific device interface. In this case, you can't use virtio-pci 
directly, then you need present virtio on the upper layer (with some 
software meditation)


>
> This is the way to do it.


Again, for debug or acceleration, if you want to do it at the virtio 
level, you need generalize those features and define them in the spec.

Maybe we can start the work on generalizing the debug facility first. 
(adding Laurent who is working on this).

Tahnks