Discussion of the VIRTIO specification
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Max Gurtovoy <mgurtovoy@nvidia.com>,
	Jason Wang <jasowang@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Cc: virtio-comment@lists.oasis-open.org, stefanha@redhat.com,
	oren@nvidia.com, parav@nvidia.com, shahafs@nvidia.com,
	eperezma@redhat.com, aadam@redhat.com, bodong@nvidia.com,
	amikheev@nvidia.com
Subject: [virtio-comment] Re: [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification
Date: Mon, 02 Aug 2021 16:51:34 +0200	[thread overview]
Message-ID: <877dh3293d.fsf@redhat.com> (raw)
In-Reply-To: <9b7ddc04-2669-0ab8-1304-1bfdf3b8e96c@nvidia.com>

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.

>>>>>
>>>>> Some features are vendor/sub-vendor specific.
>>
>>
>> Please don't do this. This breaks the efforts for making virtio as a 
>> standard and generic device.
>>
> no it doesn't.
>
> virtio already has vendor specific section.

But only as a pci-specific capability, with a very narrow scope and
constraints; that is a very far cry from what you're proposing!

>
>
>>
>>>>>
>>>>> And as mentioned, you already added it to  the spec so I guess it 
>>>>> was for a
>>>>> reason.
>>>> Well we basically just reserved a capability ID so if people add their
>>>> vendor specific ones they don't conflict with each other or 
>>>> capabilities
>>>> we'll add in the future.
>>>
>>> it shouldn't bother you as a driver maintainer. I don't think vendor 
>>> specific code should go to Linux kernel.
>>>
>>> No conflict will happen.
>>>
>>> There is no harm in having a virtio-cli tool that will pass-through 
>>> commands from command line to the device using the admin queue.
>>>
>>> All the HW devices I know have sw tool that can access it and virtio 
>>> shouldn't be different.
>>>
>>> Also, I can develop my own vendor specific user space driver for 
>>> virtio-blk for example (lets say in SPDK framework). And this driver 
>>> will be aware of this vendor specific capabilities.
>>
>>
>> That's even worse since the driver can only work for the vendor 
>> specific device which is in fact not a standard device.
>
> this is not true.

What is not true? Adding some magic vendor-specific stuff that is needed
for the device to function does make it de facto non-standard.

>
>
>>
>> For the vendor specific capability, the spec has already said that it 
>> will be only used for debugging/reporting purpose:
>>
>> """
>>
>> The optional Vendor data capability allows the device to present
>> vendor-specific data to the driver, without
>> conflicts, for debugging and/or reporting purposes,
>> and without conflicting with standard functionality.
>>
>> """
>>
>> It looks to me what you're trying to invent may violate the spec.
>>
> this is also not true.

What is not true about that? That capability has even more restrictions
than what is cited here.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


  reply	other threads:[~2021-08-02 14:51 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-26 16:52 [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification Max Gurtovoy
2021-07-26 16:52 ` [RFC PATCH v2 2/2] virtio-blk: add support for VIRTIO_F_ADMIN_VQ Max Gurtovoy
2021-07-27 12:24   ` Stefan Hajnoczi
2021-07-27 16:08     ` [virtio-comment] " Max Gurtovoy
2021-07-28  8:25       ` Stefan Hajnoczi
2021-07-27 10:27 ` [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification Stefan Hajnoczi
2021-07-27 14:28   ` [virtio-comment] " Cornelia Huck
2021-07-27 15:29     ` Max Gurtovoy
2021-07-28  8:52       ` Stefan Hajnoczi
2021-07-28 10:59         ` Max Gurtovoy
2021-07-28 13:42           ` Stefan Hajnoczi
2021-07-28 14:20             ` Max Gurtovoy
2021-07-29  8:48               ` Stefan Hajnoczi
2021-08-01 10:46                 ` [virtio-comment] " Max Gurtovoy
2021-08-02 12:58                   ` Stefan Hajnoczi
2021-07-28 12:53       ` Michael S. Tsirkin
2021-07-30  6:45         ` [virtio-comment] " Cornelia Huck
2021-07-28 12:48 ` Michael S. Tsirkin
2021-07-29 14:51   ` Max Gurtovoy
2021-07-30  7:05     ` [virtio-comment] " Cornelia Huck
2021-07-31 11:34       ` Max Gurtovoy
2021-07-31 22:26         ` Michael S. Tsirkin
2021-07-31 22:53           ` Max Gurtovoy
2021-08-01  8:16             ` Michael S. Tsirkin
2021-08-01  8:38               ` Max Gurtovoy
2021-08-02  2:17             ` Jason Wang
2021-08-02  2:19               ` Jason Wang
2021-08-02  9:54               ` Max Gurtovoy
2021-08-02 14:51                 ` Cornelia Huck [this message]
2021-08-02 15:27                   ` [virtio-comment] " Max Gurtovoy
2021-08-02 17:28                     ` Michael S. Tsirkin
2021-08-03  3:39                     ` Jason Wang
2021-08-03  8:32                       ` Max Gurtovoy
2021-08-03  9:01                         ` Jason Wang
2021-08-03  9:21                           ` Max Gurtovoy
2021-08-03 10:04                             ` [virtio-comment] " Jason Wang
2021-07-30  7:36     ` Michael S. Tsirkin
2021-07-31 11:53       ` Max Gurtovoy
2021-07-31 22:17         ` Michael S. Tsirkin
2021-07-31 23:46           ` Max Gurtovoy
2021-08-02 13:22             ` Stefan Hajnoczi
2021-08-02 14:34               ` [virtio-comment] " Cornelia Huck
2021-08-02 14:58                 ` Max Gurtovoy
2021-08-02 16:39                   ` Stefan Hajnoczi
2021-08-02 15:21             ` [virtio-comment] " Cornelia Huck
2021-08-02 16:03               ` Max Gurtovoy
2021-08-02 17:05                 ` Michael S. Tsirkin
2021-08-03  6:28                   ` [virtio-comment] " Cornelia Huck
2021-08-03  6:41                     ` Jason Wang
2021-08-03  6:51                       ` [virtio-comment] " Cornelia Huck
2021-08-03  7:55                         ` Max Gurtovoy
2021-08-03  8:55                           ` Cornelia Huck
2021-08-03  9:04                             ` Max Gurtovoy
2021-08-02  2:25   ` Jason Wang
2021-08-02  9:51     ` Max Gurtovoy
2021-08-02 17:07     ` Michael S. Tsirkin
2021-08-03  3:22       ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877dh3293d.fsf@redhat.com \
    --to=cohuck@redhat.com \
    --cc=aadam@redhat.com \
    --cc=amikheev@nvidia.com \
    --cc=bodong@nvidia.com \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=mst@redhat.com \
    --cc=oren@nvidia.com \
    --cc=parav@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=stefanha@redhat.com \
    --cc=virtio-comment@lists.oasis-open.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox