All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Parav Pandit <parav@nvidia.com>
Cc: Cornelia Huck <cohuck@redhat.com>,
	"virtio-comment@lists.oasis-open.org"
	<virtio-comment@lists.oasis-open.org>,
	"david.edmondson@oracle.com" <david.edmondson@oracle.com>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	"sburla@marvell.com" <sburla@marvell.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	Yishai Hadas <yishaih@nvidia.com>,
	Maor Gottlieb <maorg@nvidia.com>,
	Shahaf Shuler <shahafs@nvidia.com>
Subject: [virtio-dev] Re: [PATCH v10 4/4] transport-pci: Introduce group legacy group member config region access
Date: Thu, 6 Jul 2023 14:56:34 -0400	[thread overview]
Message-ID: <20230706145538-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <PH0PR12MB54817A61E0062B9BF3401836DC2CA@PH0PR12MB5481.namprd12.prod.outlook.com>

On Thu, Jul 06, 2023 at 06:53:55PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, July 6, 2023 2:48 PM
> 
> 
> > > The driver of the owner device can send a driver notification to the
> > > member device by executing
> > VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE
> > > with the \field{offset} matching \field{Queue Notify} and the
> > > \field{data} containing the virtqueue index to be notified or by
> > > performing memory or I/O write in
> > 
> > split to two sentences please. E.g.
> > Alternatively, the driver ...
> >
> Ack.
> Moving to it dedicated para, as in this first para the notification region and command is not yet introduced.
>  
> > > the any of the notification region at offset 0 supplied by the device
> > > in
> > 
> > the any is agrammatical
> > 
> > > VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO result.
> > >
> > > > Please also call out the unusual configuration where the type is
> > > > "member" and then you have the owner driver access the memory of the
> > member device.
> > > > People might be confused.
> > > >
> > > > I also think we should explain that order of entries is a hint to
> > > > driver: use the 1st entry that you can.
> > >
> > > Driver really can choose any valid entry out of the 4 that driver likes.
> > > I really don't see a need for overwriting this area as I fail to see why one will
> > expose multiple entries from the device side in reality.
> > 
> > I do: one for owner one for member.
> Yes, I am aware of choice or multi-entry. :)
> 
> > which is best for device is device specific.
> > 
> > I don't know what do you mean by "overwriting". Explaining in detail?
> :) being too verbose than needed.
> 
> > You feel like this because you spent a lot of time thinking about the area and
> > there is a specific solution is very clear in your head.
> > First readers are not like this and second they might have a different solution.
> 
> I am just saying to keep things simple. Let driver choose any entry it wants to use instead of device deciding on priority and hints etc.

Let's say driver can support both types which to choose? 
As the order is there anyway, why not just prescribe entries are
used in order?

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Parav Pandit <parav@nvidia.com>
Cc: Cornelia Huck <cohuck@redhat.com>,
	"virtio-comment@lists.oasis-open.org"
	<virtio-comment@lists.oasis-open.org>,
	"david.edmondson@oracle.com" <david.edmondson@oracle.com>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	"sburla@marvell.com" <sburla@marvell.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	Yishai Hadas <yishaih@nvidia.com>,
	Maor Gottlieb <maorg@nvidia.com>,
	Shahaf Shuler <shahafs@nvidia.com>
Subject: [virtio-comment] Re: [PATCH v10 4/4] transport-pci: Introduce group legacy group member config region access
Date: Thu, 6 Jul 2023 14:56:34 -0400	[thread overview]
Message-ID: <20230706145538-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <PH0PR12MB54817A61E0062B9BF3401836DC2CA@PH0PR12MB5481.namprd12.prod.outlook.com>

On Thu, Jul 06, 2023 at 06:53:55PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, July 6, 2023 2:48 PM
> 
> 
> > > The driver of the owner device can send a driver notification to the
> > > member device by executing
> > VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE
> > > with the \field{offset} matching \field{Queue Notify} and the
> > > \field{data} containing the virtqueue index to be notified or by
> > > performing memory or I/O write in
> > 
> > split to two sentences please. E.g.
> > Alternatively, the driver ...
> >
> Ack.
> Moving to it dedicated para, as in this first para the notification region and command is not yet introduced.
>  
> > > the any of the notification region at offset 0 supplied by the device
> > > in
> > 
> > the any is agrammatical
> > 
> > > VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO result.
> > >
> > > > Please also call out the unusual configuration where the type is
> > > > "member" and then you have the owner driver access the memory of the
> > member device.
> > > > People might be confused.
> > > >
> > > > I also think we should explain that order of entries is a hint to
> > > > driver: use the 1st entry that you can.
> > >
> > > Driver really can choose any valid entry out of the 4 that driver likes.
> > > I really don't see a need for overwriting this area as I fail to see why one will
> > expose multiple entries from the device side in reality.
> > 
> > I do: one for owner one for member.
> Yes, I am aware of choice or multi-entry. :)
> 
> > which is best for device is device specific.
> > 
> > I don't know what do you mean by "overwriting". Explaining in detail?
> :) being too verbose than needed.
> 
> > You feel like this because you spent a lot of time thinking about the area and
> > there is a specific solution is very clear in your head.
> > First readers are not like this and second they might have a different solution.
> 
> I am just saying to keep things simple. Let driver choose any entry it wants to use instead of device deciding on priority and hints etc.

Let's say driver can support both types which to choose? 
As the order is there anyway, why not just prescribe entries are
used in order?

-- 
MST


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:[~2023-07-06 18:56 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-06  4:17 [virtio-dev] [PATCH v10 0/4] admin: Access legacy registers using admin commands Parav Pandit
2023-07-06  4:17 ` [virtio-comment] " Parav Pandit
2023-07-06  4:17 ` [virtio-dev] [PATCH v10 1/4] admin: Split opcode table rows with a line Parav Pandit
2023-07-06  4:17   ` [virtio-comment] " Parav Pandit
2023-07-06  4:17 ` [virtio-dev] [PATCH v10 2/4] admin: Fix section numbering Parav Pandit
2023-07-06  4:17   ` [virtio-comment] " Parav Pandit
2023-07-06 13:39   ` [virtio-dev] " Cornelia Huck
2023-07-06 13:39     ` [virtio-comment] " Cornelia Huck
2023-07-06  4:17 ` [virtio-dev] [PATCH v10 3/4] admin: Add group member legacy register access commands Parav Pandit
2023-07-06  4:17   ` [virtio-comment] " Parav Pandit
2023-07-06 16:12   ` [virtio-dev] " Cornelia Huck
2023-07-06 16:12     ` [virtio-comment] " Cornelia Huck
2023-07-06 16:16     ` [virtio-dev] " Parav Pandit
2023-07-06 16:16       ` [virtio-comment] " Parav Pandit
2023-07-06  4:17 ` [virtio-dev] [PATCH v10 4/4] transport-pci: Introduce group legacy group member config region access Parav Pandit
2023-07-06  4:17   ` [virtio-comment] " Parav Pandit
2023-07-06 16:28   ` [virtio-dev] " Cornelia Huck
2023-07-06 16:28     ` [virtio-comment] " Cornelia Huck
2023-07-06 16:33     ` [virtio-dev] " Parav Pandit
2023-07-06 16:33       ` [virtio-comment] " Parav Pandit
2023-07-06 16:42       ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 16:42         ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 16:58         ` [virtio-dev] " Parav Pandit
2023-07-06 16:58           ` [virtio-comment] " Parav Pandit
2023-07-06 17:33           ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 17:33             ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 17:47             ` [virtio-dev] " Parav Pandit
2023-07-06 17:47               ` [virtio-comment] " Parav Pandit
2023-07-06 18:06               ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 18:06                 ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 18:16                 ` [virtio-dev] " Parav Pandit
2023-07-06 18:16                   ` [virtio-comment] " Parav Pandit
2023-07-06 18:48                   ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 18:48                     ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 18:53                     ` [virtio-dev] " Parav Pandit
2023-07-06 18:53                       ` [virtio-comment] " Parav Pandit
2023-07-06 18:56                       ` Michael S. Tsirkin [this message]
2023-07-06 18:56                         ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 19:00                         ` [virtio-dev] " Parav Pandit
2023-07-06 19:00                           ` [virtio-comment] " Parav Pandit
2023-07-06 19:42                           ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 19:42                             ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 20:21                             ` [virtio-dev] " Parav Pandit
2023-07-06 20:21                               ` [virtio-comment] " Parav Pandit
2023-07-06 20:28                               ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 20:28                                 ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 20:35                                 ` [virtio-dev] " Parav Pandit
2023-07-06 20:35                                   ` [virtio-comment] " Parav Pandit
2023-07-06 20:41                                   ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 20:41                                     ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 20:43                                     ` [virtio-dev] " Parav Pandit
2023-07-06 20:43                                       ` [virtio-comment] " Parav Pandit
2023-07-07  8:12                                       ` Cornelia Huck
2023-07-07  8:12                                         ` [virtio-dev] " Cornelia Huck
2023-07-07  8:32                                         ` Michael S. Tsirkin
2023-07-07  8:32                                           ` Michael S. Tsirkin
2023-07-06 17:38           ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 17:38             ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 16:47       ` [virtio-dev] " Cornelia Huck
2023-07-06 16:47         ` [virtio-comment] " Cornelia Huck
2023-07-06 16:52         ` [virtio-dev] " Parav Pandit
2023-07-06 16:52           ` [virtio-comment] " Parav Pandit
2023-07-06 16:39     ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 16:39       ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 16:45       ` [virtio-dev] " Parav Pandit
2023-07-06 16:45         ` [virtio-comment] " Parav Pandit
2023-07-06 16:50         ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 16:50           ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 16:54           ` [virtio-dev] " Parav Pandit
2023-07-06 16:54             ` [virtio-comment] " Parav Pandit
2023-07-06 19:00   ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 19:00     ` [virtio-comment] " Michael S. Tsirkin
2023-07-06 19:07     ` [virtio-dev] " Parav Pandit
2023-07-06 19:07       ` Parav Pandit
2023-07-06 19:59       ` [virtio-dev] " Michael S. Tsirkin
2023-07-06 19:59         ` Michael S. Tsirkin
2023-07-06 20:28         ` Parav Pandit
2023-07-06 20:28           ` [virtio-dev] " Parav Pandit

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=20230706145538-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=david.edmondson@oracle.com \
    --cc=jasowang@redhat.com \
    --cc=maorg@nvidia.com \
    --cc=parav@nvidia.com \
    --cc=sburla@marvell.com \
    --cc=shahafs@nvidia.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=yishaih@nvidia.com \
    /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.