| Message ID | 1463407735-7409-2-git-send-email-julien.grall@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Julien, This code looks good to me. Reviewed-by: Wei Chen <Wei.Chen@arm.com> On 2016/5/16 22:08, 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@arm.com> > > --- > 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); > } > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On 17/05/2016 07:40, Wei Chen wrote: > Hi Julien, Hi Wei, Please avoid top-posting on the mailing list. > This code looks good to me. Thank you for the review. > Reviewed-by: Wei Chen <Wei.Chen@arm.com> Regards,
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@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > 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 >
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); }
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@arm.com> --- 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(-)