All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Zhou Jie <zhoujie2011@cn.fujitsu.com>
Cc: izumi.taku@jp.fujitsu.com, caoj.fnst@cn.fujitsu.com,
	Chen Fan <fan.chen@easystack.cn>,
	qemu-devel@nongnu.org, mst@redhat.com
Subject: Re: [Qemu-devel] [PATCH v8 11/12] vfio: register aer resume notification handler for aer resume
Date: Mon, 11 Jul 2016 10:24:56 -0600	[thread overview]
Message-ID: <20160711102456.0baa5e26@t450s.home> (raw)
In-Reply-To: <ae6147da-eb19-2a37-7e52-c72d1c9281ea@cn.fujitsu.com>

On Sun, 10 Jul 2016 09:28:41 +0800
Zhou Jie <zhoujie2011@cn.fujitsu.com> wrote:

> Hi Alex,
> 
> > The variable clearly isn't visible to the user, so the user can know
> > whether the kernel supports this feature, but not whether the feature
> > is currently active.  Perhaps there's no way to avoid races completely,
> > but don't you expect that if we define that certain operations are
> > blocked after an error notification that a user may want some way to
> > poll for whether the block is active after a reset rather than simply
> > calling a blocked interface to probe for it?  
> Yes, I will use access blocked function, not the variable.

I don't understand what this means.
 
> > As we've discussed before, the AER notification needs to be relayed to
> > the user without delay, otherwise we only increase the gap where the
> > user might consume bogus data.  It also only seems reasonable to modify
> > the behavior of the interfaces (ie. blocking) if the user is notified,
> > which would be through the existing error notifier.  We can never
> > depend on a specific behavior from the user, we may be dealing with a
> > malicious user.
> >
> > We already disable interrupts in vfio_pci_disable() simply by calling
> > the ioctl function directly.  
> Sorry, I want to know where is vfio_pci_disable invoked.
> I can't find it in ioctl function.

You have the code, vfio_pci_disable() is invoked when the vfio device
file descriptor is released.  It's not in the ioctl, it calls the ioctl
as the user would to disable all interrupts on the device.

> > If we simply disable and re-enable interrupts as you propose,
> > how does the user deal with edge triggered
> > interrupts that may have occurred during that period?  Are they lost?
> > Should we instead leave the interrupts enabled but skip
> > eventfd_signal() in the interrupts handlers, queuing interrupts for
> > re-delivery after the device is resumed?  
> Yes, they will lost.

Is that acceptable?  This is part of the problem I have with silently
disabling interrupt delivery via the command register across reset.  It
seems more non-deterministic than properly disabling interrupts and
requiring the user to reinitialize them after error.
 
> > Or does it make more sense to
> > simply disable the interrupts as done in vfio_pci_disable() and define
> > that the user needs to re-establish interrupts before continuing after
> > an error event?  Thanks,  
> If user invoked the vfio_pci_disable by ioctl function.

I'm in no way suggesting that a user invoke vfio_pci_disable(), I'm just
trying to point out that vfio_pci_disable() already does a teardown of
interrupts, similar to what seems to be required here.

> Yes, user should re-establish interrupts before
> continuing after an error event.

So if we define that users should re-establish interrupts after an
error event, then what's the point of only doing command register
masking of the interrupts and requiring the user to both tear-down the
interrupts and re-establish them?  Thanks,

Alex

  reply	other threads:[~2016-07-11 16:25 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-27  2:12 [Qemu-devel] [PATCH v8 11/12] vfio: register aer resume notification handler for aer resume Zhou Jie
2016-05-27 16:06 ` Alex Williamson
2016-06-12  2:38   ` Zhou Jie
2016-06-20  7:41     ` Zhou Jie
2016-06-20 16:32       ` Alex Williamson
2016-06-21  2:16         ` Zhou Jie
2016-06-21  3:13           ` Alex Williamson
2016-06-21 12:41             ` Chen Fan
2016-06-21 14:44               ` Alex Williamson
2016-06-22  3:28                 ` Zhou Jie
2016-06-22  3:56                   ` Alex Williamson
2016-06-22  5:45                     ` Zhou Jie
2016-06-22  7:49                       ` Zhou Jie
2016-06-22 15:42                         ` Alex Williamson
2016-06-25  1:24                           ` Zhou Jie
2016-06-27 15:54                             ` Alex Williamson
2016-06-28  3:26                               ` Zhou Jie
2016-06-28  3:58                                 ` Alex Williamson
2016-06-28  5:27                                   ` Zhou Jie
2016-06-28 14:40                                     ` Alex Williamson
2016-06-29  8:54                                       ` Zhou Jie
2016-06-29 18:22                                         ` Alex Williamson
2016-06-30  1:45                                           ` Zhou Jie
2016-07-03  4:00                                             ` Zhou Jie
2016-07-05  1:36                                               ` Zhou Jie
2016-07-05 17:03                                                 ` Alex Williamson
2016-07-06  2:01                                                   ` Zhou Jie
2016-07-07 19:04                                                     ` Alex Williamson
2016-07-08  1:38                                                       ` Zhou Jie
2016-07-08 17:33                                                         ` Alex Williamson
2016-07-10  1:28                                                           ` Zhou Jie
2016-07-11 16:24                                                             ` Alex Williamson [this message]
2016-07-12  1:42                                                               ` Zhou Jie
2016-07-12 15:45                                                                 ` Alex Williamson
2016-07-13  1:04                                                                   ` Zhou Jie
2016-07-13  2:54                                                                     ` Alex Williamson
2016-07-13  3:33                                                                       ` Zhou Jie
2016-06-22 15:25                       ` Alex Williamson

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=20160711102456.0baa5e26@t450s.home \
    --to=alex.williamson@redhat.com \
    --cc=caoj.fnst@cn.fujitsu.com \
    --cc=fan.chen@easystack.cn \
    --cc=izumi.taku@jp.fujitsu.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=zhoujie2011@cn.fujitsu.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.