Re: [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions

[Arianna Avanzini <avanzini.arianna@gmail.com>]

On Wed, Aug 27, 2014 at 12:35:54PM -0400, Julien Grall wrote:
> Hi Arianna,

Hello!

> 
> On 24/08/14 16:29, Arianna Avanzini wrote:
> >  xen/arch/arm/p2m.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> >index b9129a0..0520f84 100644
> >--- a/xen/arch/arm/p2m.c
> >+++ b/xen/arch/arm/p2m.c
> >@@ -713,7 +713,7 @@ static int apply_p2m_changes(struct domain *d,
> >      int rc, ret;
> >      struct p2m_domain *p2m = &d->arch.p2m;
> >      lpae_t *first = NULL, *second = NULL, *third = NULL;
> >-    paddr_t addr;
> >+    paddr_t addr, orig_maddr = maddr;
> >      unsigned long cur_first_page = ~0,
> >                    cur_first_offset = ~0,
> >                    cur_second_offset = ~0;
> >@@ -844,6 +844,10 @@ out:
> >      if (third) unmap_domain_page(third);
> >      if (second) unmap_domain_page(second);
> >      if (first) unmap_domain_page(first);
> >+    if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) )
> >+        apply_p2m_changes(d, REMOVE, start_gpaddr,
> >+                          end_gpaddr, orig_maddr,
> >+                          MATTR_DEV, p2m_invalid);
> 
> s/MATTR_DEV/mattr/
> 
> Also, with unmapping the whole range you may hit your warning added in the
> previous patch in some corner case.
> It could happen when the end of range already contains another mapping. But,
> I don't think it happen often. Maybe we can live with it for now?
> 

Thank you for the correction and for pointing that corner case out. Since I
have to send a new version perhaps I'll try to handle also the case you
mentioned. Just to see if I understood things correctly: the correct behavior
would be to leave the mapping already existing at the end of the range as it
is?

I'll try to send a v12 within this week however.

Thank you,
Arianna


> Regards,
> 
> -- 
> Julien Grall