qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Yang Zhong <yang.zhong@linux.intel.com>
Cc: "Cédric Le Goater" <clg@kaod.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: About the instance_finalize callback in VFIO PCI
Date: Wed, 22 Mar 2023 12:22:27 -0600	[thread overview]
Message-ID: <20230322122227.01a51193.alex.williamson@redhat.com> (raw)
In-Reply-To: <ZBr+PNMHHVZ/mEs/@yangzhon>

On Wed, 22 Mar 2023 09:10:20 -0400
Yang Zhong <yang.zhong@linux.intel.com> wrote:

> On Wed, Mar 22, 2023 at 01:56:13PM +0100, Cédric Le Goater wrote:
> > On 3/22/23 13:28, Yang Zhong wrote:  
> > > On Tue, Mar 21, 2023 at 06:30:14PM +0100, Cédric Le Goater wrote:  
> > > > On 3/20/23 10:31, Yang Zhong wrote:  
> > > > > Hello Alex and Paolo,
> > > > > 
> > > > > There is one instance_finalize callback definition in hw/vfio/pci.c, but
> > > > > i find this callback(vfio_instance_finalize()) never be called during the
> > > > > VM shutdown with close VM or "init 0" command in guest.
> > > > > 
> > > > > The Qemu related command:
> > > > >      ......
> > > > >      -device vfio-pci,host=d9:00.0
> > > > >      ......  
> > > > 
> > > > well, the finalize op is correctly called for hot unplugged devices, using
> > > > device_add.
> > > >   
> > >     Thanks Cédric, i can use device_del command in the monitor to
> > >     trigger this instance_finalize callback function in the VFIO PCI.
> > >     thanks!  
> > 
> > yes. I think that in the shutdown case, QEMU simply relies on exit() to
> > do the cleanup. On the kernel side, unmaps, fds being closed trigger any
> > allocated resources.
> > 
> > Out of curiosity, what were you trying to achieve in the finalize op ?
> >   
>  
>  We are doing one new feature, which need this callback to do some
>  cleanup work with VFIO/iommufd kernel module. thanks!

This sounds dangerously like relying on userspace for cleanup.  Kernel
drivers need to be able to perform all cleanup themselves when file
descriptors are closed.  They must expect that userspace can be killed
at any point in time w/o an opportunity to do cleanup work.  Thanks,

Alex



  reply	other threads:[~2023-03-22 18:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-20  9:31 About the instance_finalize callback in VFIO PCI Yang Zhong
2023-03-21 17:30 ` Cédric Le Goater
2023-03-21 20:44   ` Paolo Bonzini
2023-03-22 12:39     ` Yang Zhong
2023-03-22 12:28   ` Yang Zhong
2023-03-22 12:56     ` Cédric Le Goater
2023-03-22 13:10       ` Yang Zhong
2023-03-22 18:22         ` Alex Williamson [this message]
2023-03-23  5:58           ` Yang Zhong

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=20230322122227.01a51193.alex.williamson@redhat.com \
    --to=alex.williamson@redhat.com \
    --cc=clg@kaod.org \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yang.zhong@linux.intel.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).