On 14 February 2013 13:09, Michael S. Tsirkin <m...@redhat.com> wrote: > On Thu, Feb 14, 2013 at 12:56:02PM +0000, Peter Maydell wrote: >> Up to the parent which controls the region being mapped into. > > We could just assume same priority as parent
Er, no. I mean the code in control of the parent MR sets the priority, when it calls memory_region_add_subregion_overlap(). > but what happens if it > has to be different? There are also aliases so a region > can have multiple parents. The alias has its own priority. > Presumably it will have to have > different priorities depending on what the parent does? > Here's a list of instances using priority != 0. > > hw/armv7m_nvic.c: MEMORY_PRIO_LOW); So this one I know about, and it's a good example of what I'm talking about. This function sets up a container memory region ("nvic"), and it is in total control of what is mapped into that container. Specifically, it puts in a "nvic_sysregs" background region which covers the whole 0x1000 size of the container (at an implicit priority of zero). It then layers over that an alias of the GIC registers ("nvic-gic") at a specific address and with a priority of 1 so it appears above the background region. Nobody else ever puts anything in this container, so the only thing we care about is that the priority of the nvic-gic region is higher than that of the nvic_sysregs region; and it's clear from the code that we do that. Priority is a local question whose meaning is only relevant within a particular container region, not system-wide, and having system-wide MEMORY_PRIO_ defines obscures that IMHO. >> I definitely don't like making the priority argument mandatory: >> this is just introducing pointless boilerplate for the common >> case where nothing overlaps and you know nothing overlaps. > > Non overlapping is not a common case at all. E.g. with normal PCI > devices you have no way to know nothing overlaps - addresses are guest > programmable. That means PCI is a special case :-) If the guest can program overlap then presumably PCI specifies semantics for what happens then, and there need to be PCI specific wrappers that enforce those semantics and they can call the relevant _overlap functions when mapping things. In any case this isn't a concern for the PCI *device*, which can just provide its memory regions. It's a problem the PCI *host adaptor* has to deal with when it's figuring out how to map those regions into the container which corresponds to its area of the address space. > We could add a wrapper for MEMORY_PRIO_LOWEST - will that address > your concern? Well, I'm entirely happy with the memory API we have at the moment, and I'm trying to figure out why you want to change it... >> Maybe we should take the printf() about subregion collisions >> in memory_region_add_subregion_common() out of the #if 0 >> that it currently sits in? > This is just a debugging tool, it won't fix anything. It might tell us what bits of code are currently erroneously mapping regions that overlap without using the _overlap() function. Then we could fix them. -- PMM