Hi Michal,

>> 
>> +    rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, start, end, 
>> &idx);
>> +    if ( rc < 0 )
>> +         panic("Cannot handle overlapping MPU memory protection regions\n");
> Why panic? This function is not used only at boot time and should propagate
> error to the caller, it's also within a spin lock.

Good point - I will update this to propagate the error in the next version of 
the series.

>> +    /* This API is only meant to unmap transient regions */
>> +    if ( !region_is_transient(&xen_mpumap[idx]) )
> So is this the only purpose of the transient flag? To check that 
> unmap_mm_range
> is used on the range that was mapped with map_mm_range? What would happen
> without introducing this flag? You already check for the matching attributes.
> 
> ~Michal
> 

Yes this is the purpose of the transient flag - we want to ensure that a call 
to unmap_mm_range only destroys a mapping that was created by a matching call 
to map_mm_range. Due to the fact that map_mm_range may not create a mapping in 
the instance that one already exists - `/* Already mapped with same attributes 
*/` - we need this check to ensure that unmap_mm_range will not destroy a 
pre-existing mapping.

Many thanks,
Hari


Reply via email to