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-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, 8 Oct 2021 07:18:15 -0400	[thread overview]
Message-ID: <20211008070712-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <PH0PR12MB5481E5C53991AE2A98AA7D9FDCB29@PH0PR12MB5481.namprd12.prod.outlook.com>

On Fri, Oct 08, 2021 at 10:51:02AM +0000, Parav Pandit wrote:
> > That's why I ask: why do we bother? What's wrong with just waiting forever or
> > until user gets tired of this and cancels with CTRL-C?
> 
> Today, device removal of the device gets stuck for the device which didn't finish the reset, because its waiting for ever.
> 
> modprobe to my knowledge cannot be Ctrl-C. In another scenario, device probing of hot plug device occurs by hotplug driver in a workqueue context.

Frankly I'm not sure we need to worry about esoterica like
hotplug working when device can't get out of reset. If guest is
more or less alive that's already an achievement.

> It doesn't sound right to pass the burden to the user to invent some kind of ctrl-C cancel operation in hotplug drivers.

Oh interesting. Thanks for providing the motivation.

So if it's device removal you are after to fix,
then the proposed spec won't be enough I suspect, since
there's no specific time when we can be sure DMA won't
happen anymore. Just giving up on device isn't possible,
if you do you risk corrupting guest memory which seems scarier
than just blocking hotplug.

I guess with this in mind, what would be needed is a more fine-grained
approach. E.g. driver writes 0 to reset, device returns 1 to indicate
reset in progress, at that point it can promise that DMA/interrupts
won't happen any longer, so driver can go away.




> > Is there a use-case where that's not good enough?
> A guest has got one good and another device that encountered a fault.
> Due to this faulty device which is unable to reset, is blocking other operations in the system.

It's pretty uncommon for guests to actually have devices
they don't really need to function normally.

> > 
> > 
> > > And this is probably yet another good reason to define migratable bits
> > > of a virtio device in the live migration spec extension.
> > 
> > "migratable bits" being what? non-guest visible device state? Sure, would be
> > great to have.  Don't think it will help in this instance.
> > 
> > --
> > MST


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


  reply	other threads:[~2021-10-08 11:18 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                 ` Michael S. Tsirkin [this message]
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
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=20211008070712-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=mgurtovoy@nvidia.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.