On Mon, Aug 18, 2014 at 8:23 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 15 August 2014 08:17, Peter Crosthwaite <peter.crosthwa...@xilinx.com> > wrote: >> In a5e1cbc80e88ed7d73b3fcb46053a3ba167293fc the enforcement of Memory >> collisions was disabled. This means that the MemoryRegion map_overlap >> state is unused. Remove it completely. >> >> The commit mentions that it should be fixed, but we have been living >> happily-every-after since removal of the check so it's probably >> unneeded complication. > > Um. I think in general colliding memory regions imply a bug in the > machine model which is creating them. We have the collisions > disabled because nobody's got round to fixing this bug in the > PC model yet and it happens to be harmless there. I'm not > really convinced that yanking out the check code is better than > fixing the PC model... > >> If we were to repair this, a simpler and more effective check would be >> to only assert collisions between same-priority regions. The fact that >> colliding memory regions may-overlap is then left as implicit by the >> fact that they have different priorities. > > I'm not sure your suggestion here would work, because priorities > are only significant relative to other regions within the same > container, whereas collisions can occur between two regions > which don't have the same parent container and whose priorities are > therefore not comparable. (For instance, consider [ A [ B C ] ] > where A and B end up overlapping.) >
But that is not a problem that is solved by the old may_overlap flag is it? The check deleted here is not hierarchy aware so we have never been able to detect that case. I think we should take a "clean slate" approach on the implementation of the collision detection. Big change is needed to get the check in the right place in code, whether it's same-priority based or using may_overlap. Regards, Peter > thanks > -- PMM >
