qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).