Re: [Xen-devel] [for-4.7 1/2] xen/arm: p2m: apply_p2m_changes: Do not undo more than necessary

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 16 May 2016, Julien Grall wrote:
> Since commit 4b25423a "arch/arm: unmap partially-mapped memory regions",
> Xen has been undoing the P2M mappings when an error occurred during
> insertion or memory allocation.
> 
> The function apply_p2m_changes can work with region not-aligned to a
> block size (2MB, 1G) or page size (4K). The mapping will be done by
> splitting the region in a set of regions aligned to the size supported
> by the page table.
> 
> The mapping of a region could fail when it is not possible to allocate
> memory for an intermediate table (i.e a new or when shattering a block).
> 
> When the mapping is undone, the end of the region is computed using the
> base address of the current region and the size of the failing level.
> However the failing level may not be the leaf one, therefore unrelated
> entries will be removed.
> 
> Fix it by removing the mapping from the start address up to the last
> region that has been successfully mapped.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


>     This patch is a bug fix for Xen 4.7 and candidate for backporting
>     up to Xen 4.5. Without this patch, Xen may undo mapping which are
>     not part of the region mapped when memory allocation has failed.
> 
>     Note that Xen 4.7 has code to remove empty translation table (see
>     commit de5162b "xen/arm: p2m: Remove translation table when it's
>     empty"), however with this patch those tables will not be removed
>     in case of failure. This will be fixed after the release as
>     the change will be too intrusive for Xen 4.7.
> ---
>  xen/arch/arm/p2m.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index db21433..68c67b0 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1189,13 +1189,10 @@ out:
>      {
>          BUG_ON(addr == end_gpaddr);
>          /*
> -         * addr keeps the address of the last successfully-inserted mapping,
> -         * while apply_p2m_changes() considers an address range which is
> -         * exclusive of end_gpaddr: add level_size to addr to obtain the
> -         * right end of the range
> +         * addr keeps the address of the end of the last 
> successfully-inserted
> +         * mapping.
>           */
> -        apply_p2m_changes(d, REMOVE,
> -                          start_gpaddr, addr + level_sizes[level], 
> orig_maddr,
> +        apply_p2m_changes(d, REMOVE, start_gpaddr, addr, orig_maddr,
>                            mattr, 0, p2m_invalid, d->arch.p2m.default_access);
>      }
>  
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel