All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Parav Pandit <parav@nvidia.com>, "Michael S. Tsirkin" <mst@redhat.com>
Cc: "virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	Max Gurtovoy <mgurtovoy@nvidia.com>,
	Shahaf Shuler <shahafs@nvidia.com>, Oren Duer <oren@nvidia.com>
Subject: [virtio-dev] RE: [PATCH v2] Add device reset timeout field
Date: Fri, 15 Oct 2021 08:51:53 +0200	[thread overview]
Message-ID: <87czo6vkza.fsf@redhat.com> (raw)
In-Reply-To: <DM8PR12MB54806B9EEECE8920F450F86BDCB99@DM8PR12MB5480.namprd12.prod.outlook.com>

On Fri, Oct 15 2021, Parav Pandit <parav@nvidia.com> wrote:

>> From: Michael S. Tsirkin <mst@redhat.com>
>> Sent: Friday, October 15, 2021 3:59 AM
>> 
>> On Thu, Oct 14, 2021 at 05:35:37PM +0000, Parav Pandit wrote:
>> > Below changes are good for v3?
>> > 1. driver should use device reset time during initialization stage
>> 
>> How does driver identify this though?
> Existence of device_reset_timeout field in struct virtio_pci_common_cfg indicates that this field exists.
> If device support it, it will place non zero value and driver knows that this field should be used.

But how does the driver know? The very first step in the initialization
sequence is "reset the device", before it may read the config space.

>
>> 
>> > 2. remove feature bit as feature bits are only readable after reset is
>> > completed 3. device reset timeout field of zero indicates that device doesn't
>> support it.
>> 
>> I'm not sure about 3. I think each transport will need its own way to do it.
>> 
> For pci a value of zero indicates it isn't supported.
> For mmio DeviceResetTimeout at offset 0x04c indicates same.
> Currently only these 2 transports have the use.

I don't think we want to force all transports, including not yet
existing ones, to use that mechanism. They might as well use a command
to retrieve the value and fail the command, for example.

>
>> So I propose: maybe a capability like this, with a timeout field?
> Do you mean a new capability like say VIRTIO_PCI_DEVICE_TIMEOUT like VIRTIO_PCI_CAP_COMMON_CFG?
> This will contain one or more timeout? For example with his proposal it contains only device reset timeout.
> Later same capability will be further extended to contains command timeout too? Yes?
>
>> And within VMs, we can just do without, since it got out of reset once it will
>> surely get out of reset again...
> Yes, VM might not need it. It is really the HV's choice to implement and not part of the virtio spec.
> Our internal cloud passthrough a PF to the VM.
> It is probably better to let HV to choose if they want to do ctrl+c or have timeout.

But you want it in the spec, don't you? Confused.


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


  parent reply	other threads:[~2021-10-15  6:52 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-06 14:10 [PATCH v2] Add device reset timeout field Parav Pandit
2021-10-06 15:22 ` Michael S. Tsirkin
2021-10-06 16:11   ` Parav Pandit
2021-10-06 20:53     ` Michael S. Tsirkin
2021-10-07  3:42       ` Parav Pandit
2021-10-07 16:10         ` [virtio-dev] " Cornelia Huck
2021-10-07 17:58           ` Parav Pandit
2021-10-08 10:00             ` [virtio-dev] " Cornelia Huck
2021-10-08 10:19               ` Parav Pandit
2021-10-08 10:12             ` Michael S. Tsirkin
2021-10-08 10:51               ` Parav Pandit
2021-10-08 11:18                 ` [virtio-dev] " Michael S. Tsirkin
2021-10-08 12:55                   ` Parav Pandit
2021-10-08 10:44         ` Michael S. Tsirkin
2021-10-08 10:59           ` Parav Pandit
2021-10-08 11:21             ` Michael S. Tsirkin
2021-10-08 11:45               ` Parav Pandit
2021-10-08 11:47               ` [virtio-dev] " Cornelia Huck
2021-10-08 12:12                 ` Parav Pandit
2021-10-08 12:57                   ` Michael S. Tsirkin
2021-10-08 13:23                     ` Parav Pandit
2021-10-08 23:09                       ` Michael S. Tsirkin
2021-10-11 14:29                         ` Parav Pandit
2021-10-11 14:59                           ` [virtio-dev] " Cornelia Huck
2021-10-11 15:44                             ` Parav Pandit
2021-10-11 16:00                               ` Michael S. Tsirkin
2021-10-12  8:51                                 ` Parav Pandit
2021-10-12  9:01                                   ` Michael S. Tsirkin
2021-10-12  9:12                                     ` Parav Pandit
2021-10-14 17:35                                       ` Parav Pandit
2021-10-14 22:28                                         ` Michael S. Tsirkin
2021-10-15  4:36                                           ` Parav Pandit
2021-10-15  5:15                                             ` [virtio-dev] " Jason Wang
2021-10-15  5:20                                               ` Parav Pandit
2021-10-15  6:40                                                 ` Jason Wang
2021-10-15  6:42                                                 ` Jason Wang
2021-10-15  6:48                                                   ` Parav Pandit
2021-10-15  7:02                                                     ` Jason Wang
2021-10-15  8:21                                                       ` Parav Pandit
2021-10-15  8:42                                                         ` Jason Wang
2021-10-22  7:20                                                           ` Parav Pandit
2021-10-25  5:41                                                             ` Jason Wang
2021-10-25  6:11                                                               ` Parav Pandit
2021-10-26  4:03                                                                 ` Jason Wang
2021-10-27  8:04                                                                   ` Parav Pandit
2021-10-27  8:26                                                                     ` Michael S. Tsirkin
2021-10-28  4:01                                                                       ` Parav Pandit
2021-10-28  5:50                                                                         ` Michael S. Tsirkin
2021-10-28  6:06                                                                           ` Parav Pandit
2021-10-15  6:51                                             ` Cornelia Huck [this message]
2021-10-15  8:09                                               ` Parav Pandit
2021-10-15  9:25                                                 ` [virtio-dev] " Cornelia Huck
2021-10-22  6:29                                                   ` Parav Pandit
2021-10-11 16:22                               ` [virtio-dev] " Cornelia Huck
2021-10-12 10:35                                 ` 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=87czo6vkza.fsf@redhat.com \
    --to=cohuck@redhat.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=mst@redhat.com \
    --cc=oren@nvidia.com \
    --cc=parav@nvidia.com \
    --cc=shahafs@nvidia.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.