Re: [Qemu-devel] [PATCH 0/3] Remove may_overlap from MemoryRegion and use priorities instead

[Peter Maydell]

On 18 February 2013 23:12, Alexey Korolev <address@hidden> wrote:
> At the moment may_overlap flag of MemoryRegion structure is used
> inconsistently, and ignored by the address range assignment process.
> In fact may_overlap flag can be fully substituted with priority
> of the MemoryRegion structure. Also usage of MemoryRegion
> priorities is a bit confusing. Exisiting priorities can take values
> 0,1,2 or even 1000.

So, we just had a conversation about memory region overlaps:
http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg02044.html

You don't give any motivation in your cover letter for making
this change; do you have an underlying problem you're trying
to fix?

(As far as I can tell the problem discussed in the thread I link
to above is simply "the PC model isn't actually modelling the
overlap semantics correctly and needs to be fixed".)

Patch series which update the memory API along the lines of
"here's a problem; we could solve it with the API like this
but that is obviously rather cumbersome; this minor API
change makes the solution much cleaner" would be more
convincing IMHO. [For example, you could make a case along
these lines for the suggestion MST makes of having priority
be signed rather than unsigned so the non-overlap default is
middle of the range rather than an extreme edge.]

On this specific suggested change:
> use only the memory_region_add_subregion function to add a
> subregion to a container, and optionally use
> memory_region_set_priority() to change priority from the default

I think this is a bad idea, because priority of a region
exists only conditionally and locally because it has been
added as a subregion of a container. It makes no sense to
set the priority of the subregion separately while it is still
floating about unattached to any container. Having the setting
of the priority happen exactly as the subregion is put into
the container makes this clear.

And the correct answer to "qemu memory manager considers every
memory region as overlappable" is that we should actually
enable the error path which warns about overlapping regions
which aren't marked as overlappable, and fix the bugs that
reveals.

-- PMM