All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.