qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: mst@redhat.com, aik@ozlabs.ru, qemu-devel@nongnu.org,
	agraf@suse.de, alex.williamson@redhat.com, pbonzini@redhat.com
Subject: Re: [Qemu-devel] [PATCH 6/8] memory: Sanity check that no listeners remain on a destroyed AddressSpace
Date: Mon, 13 May 2013 21:48:17 +1000	[thread overview]
Message-ID: <20130513114817.GB14944@truffula.fritz.box> (raw)
In-Reply-To: <CAFEAcA9PvgzCoTdcYOKLSGpaMvPZzN2dfrPiRvnXdhtPfy+k=w@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

On Mon, May 13, 2013 at 12:10:10PM +0100, Peter Maydell wrote:
> On 13 May 2013 11:54, David Gibson <david@gibson.dropbear.id.au> wrote:
> > At the moment, most AddressSpace objects last as long as the guest system
> > in practice, but that could well change in future.  In addition, for VFIO
> > we will be introducing some private per-AdressSpace information, which must
> > be disposed of before the AddressSpace itself is destroyed.
> >
> > To reduce the chances of subtle bugs in this area, this patch adds
> > asssertions to ensure that when an AddressSpace is destroyed, there are no
> > remaining MemoryListeners using that AS as a filter.
> 
> Hmm, is this the ideal semantics? Typically the owner of the
> MemoryListener isn't the owner of the AddressSpace so it isn't
> necessarily in a position to guarantee that it can unregister
> the listener before the address space is destroyed. In fact
> as the listener API is currently documented, the filter
> argument is just an optimisation to save the callbacks having
> to filter out irrelevant information themselves.

If so, then it's broken by design.  There's no guarantee that after an
AddressSpace is destroyed another one won't be created at the same
address (in fact, depending on your malloc() implementation, it could
be very likely).  So references by pointer to an object *must* be
removed before the object itself is freed.

> Perhaps an "address space being destroyed" callback for
> listeners would be better? Then VFIO could just do the
> necessary disposal operations automatically when the AS
> goes away.

I was originally going to do that instead, but none of the things I
have planned immediately need it, since I think other lifetime
constraints will sort things out (chiefly that PCI devices must be
removed before their host bridge can, which will control the
AddressSpace lifetime).  This patch is so that if I'm wrong about
that, it will show up clearly, at which point we can implement such a
callback.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

  reply	other threads:[~2013-05-13 11:50 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-13 10:54 [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited David Gibson
2013-05-13 10:54 ` [Qemu-devel] [PATCH 1/8] iommu: Fix compile error in ioapic.c David Gibson
2013-05-13 12:14   ` Paolo Bonzini
2013-05-13 10:54 ` [Qemu-devel] [PATCH 2/8] pci: Don't del_subgregion on a non subregion David Gibson
2013-05-13 12:14   ` Paolo Bonzini
2013-05-13 10:54 ` [Qemu-devel] [PATCH 3/8] pci: Rework PCI iommu lifetime assumptions David Gibson
2013-05-13 12:15   ` Paolo Bonzini
2013-05-13 10:54 ` [Qemu-devel] [PATCH 4/8] pci: Use AddressSpace rather than MemoryRegion to represent PCI DMA space David Gibson
2013-05-13 12:16   ` Paolo Bonzini
2013-05-13 10:54 ` [Qemu-devel] [PATCH 5/8] pci: Introduce helper to retrieve a PCI device's DMA address space David Gibson
2013-05-13 10:54 ` [Qemu-devel] [PATCH 6/8] memory: Sanity check that no listeners remain on a destroyed AddressSpace David Gibson
2013-05-13 11:10   ` Peter Maydell
2013-05-13 11:48     ` David Gibson [this message]
2013-05-13 12:07       ` Peter Maydell
2013-05-13 12:19         ` Paolo Bonzini
2013-05-13 12:57           ` David Gibson
2013-05-13 10:54 ` [Qemu-devel] [PATCH 7/8] vfio: Introduce VFIO address spaces David Gibson
2013-05-13 21:32   ` Alex Williamson
2013-05-14  1:00     ` David Gibson
2013-05-13 10:54 ` [Qemu-devel] [PATCH 8/8] vfio: Create VFIOAddressSpace objects as needed David Gibson
2013-05-13 21:33   ` Alex Williamson
2013-05-14  1:58     ` David Gibson
2013-05-13 12:23 ` [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited Paolo Bonzini
2013-05-13 13:13   ` David Gibson
2013-05-13 13:30     ` Paolo Bonzini
2013-05-14  2:39       ` David Gibson
2013-05-14  9:58         ` Paolo Bonzini
2013-05-15  3:55           ` David Gibson

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=20130513114817.GB14944@truffula.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --cc=alex.williamson@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.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 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).