Re: [Qemu-devel] [PATCH memory v1 1/1] memory: remove may_overlap property

[Peter Crosthwaite]

On Mon, Aug 18, 2014 at 8:23 AM, Peter Maydell <address@hidden> wrote:
> On 15 August 2014 08:17, Peter Crosthwaite <address@hidden> 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
>