Re: [Qemu-devel] [PATCH 3/3] docs: clarify memory region lifecycle

[Matthew Rosato]

On 02/13/2015 09:57 AM, Paolo Bonzini wrote:
> Now that objects actually obey the rules, document them.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

Reviewed-by: Matthew Rosato <address@hidden>

> ---
>  docs/memory.txt | 74 
> ++++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 58 insertions(+), 16 deletions(-)
> 
> diff --git a/docs/memory.txt b/docs/memory.txt
> index b12f1f0..2ceb348 100644
> --- a/docs/memory.txt
> +++ b/docs/memory.txt
> @@ -73,17 +73,66 @@ stability.
>  Region lifecycle
>  ----------------
> 
> -A region is created by one of the constructor functions 
> (memory_region_init*())
> -and attached to an object.  It is then destroyed by object_unparent() or 
> simply
> -when the parent object dies.
> +A region is created by one of the memory_region_init*() functions and
> +attached to an object, which acts as its owner or parent.  QEMU ensures
> +that the owner object remains alive as long as the region is visible to
> +the guest, or as long as the region is in use by a virtual CPU or another
> +device.  For example, the owner object will not die between an
> +address_space_map operation and the corresponding address_space_unmap.
> 
> -In between, a region can be added to an address space
> -by using memory_region_add_subregion() and removed using
> -memory_region_del_subregion().  Destroying the region implicitly
> -removes the region from the address space.
> +After creation, a region can be added to an address space or a
> +container with memory_region_add_subregion(), and removed using
> +memory_region_del_subregion().
> 
> -Region attributes may be changed at any point; they take effect once
> -the region becomes exposed to the guest.
> +Various region attributes (read-only, dirty logging, coalesced mmio,
> +ioeventfd) can be changed during the region lifecycle.  They take effect
> +as soon as the region is made visible.  This can be immediately, later,
> +or never.
> +
> +Destruction of a memory region happens automatically when the owner
> +object dies.
> +
> +If however the memory region is part of a dynamically allocated data
> +structure, you should call object_unparent() to destroy the memory region
> +before the data structure is freed.  For an example see VFIOMSIXInfo
> +and VFIOQuirk in hw/vfio/pci.c.
> +
> +You must not destroy a memory region as long as it may be in use by a
> +device or CPU.  In order to do this, as a general rule do not create or
> +destroy memory regions dynamically during a device's lifetime, and only
> +call object_unparent() in the memory region owner's instance_finalize
> +callback.  The dynamically allocated data structure that contains the
> +memory region then should obviously be freed in the instance_finalize
> +callback as well.
> +
> +If you break this rule, the following situation can happen:
> +
> +- the memory region's owner had a reference taken via memory_region_ref
> +  (for example by address_space_map)
> +
> +- the region is unparented, and has no owner anymore
> +
> +- when address_space_unmap is called, the reference to the memory region's
> +  owner is leaked.
> +
> +
> +There is an exception to the above rule: it is okay to call
> +object_unparent at any time for an alias or a container region.  It is
> +therefore also okay to create or destroy alias and container regions
> +dynamically during a device's lifetime.
> +
> +This exceptional usage is valid because aliases and containers only help
> +QEMU building the guest's memory map; they are never accessed directly.
> +memory_region_ref and memory_region_unref are never called on aliases
> +or containers, and the above situation then cannot happen.  Exploiting
> +this exception is rarely necessary, and therefore it is discouraged,
> +but nevertheless it is used in a few places.
> +
> +For regions that "have no owner" (NULL is passed at creation time), the
> +machine object is actually used as the owner.  Since instance_finalize is
> +never called for the machine object, you must never call object_unparent
> +on regions that have no owner, unless they are aliases or containers.
> +
> 
>  Overlapping regions and priority
>  --------------------------------
> @@ -215,13 +264,6 @@ BAR containing MMIO registers is mapped after it.
>  Note that if the guest maps a BAR outside the PCI hole, it would not be
>  visible as the pci-hole alias clips it to a 0.5GB range.
> 
> -Attributes
> -----------
> -
> -Various region attributes (read-only, dirty logging, coalesced mmio, 
> ioeventfd)
> -can be changed during the region lifecycle.  They take effect once the region
> -is made visible (which can be immediately, later, or never).
> -
>  MMIO Operations
>  ---------------
>