All of lore.kernel.org
 help / color / mirror / Atom feed
From: Parav Pandit <parav@nvidia.com>
To: Jason Wang <jasowang@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
Cc: Max Gurtovoy <mgurtovoy@nvidia.com>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	Shahaf Shuler <shahafs@nvidia.com>, Oren Duer <oren@nvidia.com>,
	"stefanha@redhat.com" <stefanha@redhat.com>
Subject: RE: [PATCH 2/5] Introduce VIRTIO_F_ADMIN_VQ_INDIRECT_DESC/VIRTIO_F_ADMIN_VQ_IN_ORDER
Date: Wed, 19 Jan 2022 04:48:13 +0000	[thread overview]
Message-ID: <PH0PR12MB5481D5BD8BB8CFDEFA218D2FDC599@PH0PR12MB5481.namprd12.prod.outlook.com> (raw)
In-Reply-To: <15063a94-85ae-fd1a-6ff1-458a039d1def@redhat.com>


> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, January 19, 2022 9:33 AM
> 
> 
> It it means IMS, there's already a proposal[1] that introduce MSI commands
> via the admin virtqueue. And we had similar requirement for virtio-MMIO[2]
> and managed device or SF [3], so I would rather to introduce IMS (need a
> better name though) as a basic facility instead of tie it to any specific
> transport.
> 
IMS of [1] is a interrupt configuration by the virtio driver for the device is it driving, which needs a queue.
So regardless of the device type as PCI PF/VF/SF/ADI, there is desire to have a generic admin queue not attached to device type.
And AQ in this proposal exactly serves this purpose.

Device configuring its own IMS vector vs PCI PF configuring VF's MSI-X max vector count are two different functionality.
Both of these commands can ride on a generic queue.
However the queue is not same, because
PF owns its own admin queue (for vf msix config), 
VF or SF operates its own admin queue (for IMS config).

So a good example is,
1. PCI PF configures 8 MSI-X or 16 IMS vectors for the VF using PF_AQ in HV.
2. PCI VF when using IMS configures, IMS data, vector, mask etc using VF_AQ in GVM.
Both the functions will have AQ feature bit set.

Fair enough, so we have more users of admin queue than just MSI-X config.

> 
> > 2. AQ to follows IN_ORDER and INDIRECT_DESC negotiation like rest of
> > the queues 3. Update commit log to describe why config space is not
> > chosen (scale, on-die registers, uniform way to handle all aq cmds)
> 
> 
> I fail to understand the scale/registeres issues. With the one of my previous
> proposal (device selector), technically we don't even need any config space or
> BAR for VF or SF by multiplexing the registers for PF.
>
Scale issue is: when you want to create, query, manipulate hundreds of objects, having shared MMIO register or configuration register, will be too slow.
And additionally such register set doesn't scale to allow sharing large number of bytes as DMA cannot be done.

From physical device perspective, it doesn’t scale because device needs to have those resources ready to answer on MMIO reads and for hundreds to thousand of devices it just cannot do it.
This is one of the reason for birth of IMS.

> I do see one advantage is that the admin virtqueue is transport independent
> (or it could be used as a transport).
> 
I am yet to read the transport part from [1].

> 
> > 4. Improve documentation around msix config to link to sriov section of virtio
> spec
> > 5. Describe error that if VF is bound to the device, admin commands
> targeting VF can fail, describe this error code
> >
> > Did I miss anything?
> >
> > Yet to receive your feedback on group, if/why is it needed and, why/if it must
> be in this proposal, what pieces prevents it do as follow-on.
> >
> > Cornelia, Jason,
> > Can you please review current proposal as well before we revise v2?
> 
> 
> If I understand correctly, most of the features (except for the admin
> virtqueue in_order stuffs) are not specific to the admin virtqueue. As
> discussed in the previous versions, I still think it's better:
> 
> 1) adding sections in the basic device facility or data structure for
> provisioning and MSI
> 2) introduce admin virtqueue on top as an device interface for those
> features
>
I didn't follow your suggestion. Can you please explain?
Specifically "data structure for provisioning and MSI"..
 
> The leaves the chance for future extensions to allow those features to
> be used by transport specific interface which will benefit for
> 
AQ allows communication (command, response) between driver and device in transport independent way.
Sometimes it query/set transport specific fields like MSI-X vectors of VF.
Sometimes device configure its on IMS interrupt.
Something else in future.
So it is really a generic request-response queue.

> [1]
> https://lists.oasis-open.org/archives/virtio-comment/202108/msg00025.html

  reply	other threads:[~2022-01-19  4:48 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13 14:50 [PATCH v1 0/5] VIRTIO: Provision maximum MSI-X vectors for a VF Max Gurtovoy
2022-01-13 14:50 ` [PATCH 1/5] Add virtio Admin Virtqueue specification Max Gurtovoy
2022-01-13 17:53   ` Michael S. Tsirkin
2022-01-17  9:56     ` Max Gurtovoy
2022-01-17 21:30       ` Michael S. Tsirkin
2022-01-18  3:22         ` Parav Pandit
2022-01-18  6:17           ` Michael S. Tsirkin
2022-01-18  7:57             ` Parav Pandit
2022-01-18  8:05               ` Michael S. Tsirkin
2022-01-18  8:23                 ` Parav Pandit
2022-01-18 10:26                   ` Michael S. Tsirkin
2022-01-18 10:30                     ` Parav Pandit
2022-01-18 10:41                       ` Michael S. Tsirkin
2022-01-19  3:04         ` Jason Wang
2022-01-19  8:11           ` Michael S. Tsirkin
2022-01-25  3:35             ` Jason Wang
2022-01-17 14:12     ` Parav Pandit
2022-01-17 22:03       ` Michael S. Tsirkin
2022-01-18  3:36         ` Parav Pandit
2022-01-18  7:07           ` Michael S. Tsirkin
2022-01-18  7:14             ` Parav Pandit
2022-01-18  7:20               ` Michael S. Tsirkin
2022-01-19 11:33                 ` Max Gurtovoy
2022-01-19 12:21                   ` Parav Pandit
2022-01-19 14:47                     ` Max Gurtovoy
2022-01-19 15:38                       ` Michael S. Tsirkin
2022-01-19 15:47                         ` Max Gurtovoy
2022-01-13 14:51 ` [PATCH 2/5] Introduce VIRTIO_F_ADMIN_VQ_INDIRECT_DESC/VIRTIO_F_ADMIN_VQ_IN_ORDER Max Gurtovoy
2022-01-13 15:33   ` Michael S. Tsirkin
2022-01-13 17:07     ` Max Gurtovoy
2022-01-13 17:25       ` Michael S. Tsirkin
2022-01-17 13:59         ` Parav Pandit
2022-01-17 22:14           ` Michael S. Tsirkin
2022-01-18  4:44             ` Parav Pandit
2022-01-18  6:23               ` Michael S. Tsirkin
2022-01-18  6:32                 ` Parav Pandit
2022-01-18  6:54                   ` Michael S. Tsirkin
2022-01-18  7:07                     ` Parav Pandit
2022-01-18  7:12                       ` Michael S. Tsirkin
2022-01-18  7:30                         ` Parav Pandit
2022-01-18  7:40                           ` Michael S. Tsirkin
2022-01-19  4:21                             ` Jason Wang
2022-01-19  9:30                               ` Michael S. Tsirkin
2022-01-25  3:39                                 ` Jason Wang
2022-01-18 10:38                           ` Michael S. Tsirkin
2022-01-18 10:50                             ` Parav Pandit
2022-01-18 15:09                               ` Michael S. Tsirkin
2022-01-18 17:17                                 ` Parav Pandit
2022-01-19  7:20                                   ` Michael S. Tsirkin
2022-01-19  8:15                                     ` [virtio-dev] " Parav Pandit
2022-01-19  8:21                                       ` Michael S. Tsirkin
2022-01-19 10:10                                         ` Parav Pandit
2022-01-19 16:40                                           ` Michael S. Tsirkin
2022-01-19 17:07                                             ` Parav Pandit
2022-01-18  7:13                       ` Michael S. Tsirkin
2022-01-18  7:21                         ` Parav Pandit
2022-01-18  7:37                           ` Michael S. Tsirkin
2022-01-19  4:03                       ` Jason Wang
2022-01-19  4:48                         ` Parav Pandit [this message]
2022-01-19 20:25                           ` Parav Pandit
2022-01-25  3:45                             ` Jason Wang
2022-01-25  4:07                               ` Parav Pandit
2022-01-25  3:29                           ` Jason Wang
2022-01-25  3:52                             ` Parav Pandit
2022-01-25 10:59                               ` Max Gurtovoy
2022-01-25 12:09                                 ` Michael S. Tsirkin
2022-01-26 13:29                                   ` Parav Pandit
2022-01-26 14:11                                     ` Michael S. Tsirkin
2022-01-27  3:49                                       ` Parav Pandit
2022-01-27 13:05                                         ` Michael S. Tsirkin
2022-01-27 13:25                                           ` [virtio-dev] " Parav Pandit
2022-01-28  4:35                                     ` Jason Wang
2022-01-26  7:03                                 ` Jason Wang
2022-01-26  9:27                                   ` Max Gurtovoy
2022-01-26  9:34                                     ` Jason Wang
2022-01-26  9:45                                       ` Max Gurtovoy
2022-01-27  3:46                                         ` Jason Wang
2022-01-26  5:04                               ` Jason Wang
2022-01-26  5:26                                 ` Parav Pandit
2022-01-26  5:45                                   ` Jason Wang
2022-01-26  5:58                                     ` Parav Pandit
2022-01-26  6:06                                       ` Jason Wang
2022-01-26  6:24                                         ` Parav Pandit
2022-01-26  6:54                                           ` Jason Wang
2022-01-26  8:09                                             ` Parav Pandit
2022-01-26  9:07                                               ` Jason Wang
2022-01-26  9:47                                                 ` Parav Pandit
2022-01-13 14:51 ` [PATCH 3/5] virtio-blk: add support for VIRTIO_F_ADMIN_VQ Max Gurtovoy
2022-01-13 18:24   ` Michael S. Tsirkin
2022-01-13 14:51 ` [PATCH 4/5] virtio-net: " Max Gurtovoy
2022-01-13 17:56   ` Michael S. Tsirkin
2022-01-16  9:47     ` Max Gurtovoy
2022-01-16 16:45       ` Michael S. Tsirkin
2022-01-17 14:07       ` Parav Pandit
2022-01-17 22:22         ` Michael S. Tsirkin
2022-01-18  2:18           ` Jason Wang
2022-01-18  5:25             ` Michael S. Tsirkin
2022-01-19  4:16               ` Jason Wang
2022-01-19  9:26                 ` Michael S. Tsirkin
2022-01-25  3:53                   ` Jason Wang
2022-01-25  7:19                     ` Michael S. Tsirkin
2022-01-26  5:49                       ` Jason Wang
2022-01-26  7:02                         ` Michael S. Tsirkin
2022-01-26  7:10                           ` Jason Wang
2022-01-13 14:51 ` [PATCH 5/5] Add support for dynamic MSI-X vector mgmt for VFs Max Gurtovoy
2022-01-13 18:20   ` Michael S. Tsirkin
2022-01-18 10:38   ` Michael S. Tsirkin
2022-01-13 18:32 ` [PATCH v1 0/5] VIRTIO: Provision maximum MSI-X vectors for a VF Michael S. Tsirkin
2022-01-17 10:00   ` Shahaf Shuler
2022-01-17 21:41     ` Michael S. Tsirkin

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=PH0PR12MB5481D5BD8BB8CFDEFA218D2FDC599@PH0PR12MB5481.namprd12.prod.outlook.com \
    --to=parav@nvidia.com \
    --cc=cohuck@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=mst@redhat.com \
    --cc=oren@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=stefanha@redhat.com \
    --cc=virtio-dev@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.