Re: [Xen-devel] [PATCH 3/3] xen: arm: correctly handle removing a subset of a superpage mapping.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-07-14 at 19:34 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 07/14/2014 05:27 PM, Ian Campbell wrote:
> > This can be exercised for example via ballooning which will remove 4K
> > regions fromanywhere in the address space.
> 
> Missing space between "from" and "anywhere"
> 
> > 
> > Reported-by: Julien Grall <julien.grall@xxxxxxxxxx>
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >  xen/arch/arm/p2m.c |   29 +++++++++++++++++++++++++++--
> >  1 file changed, 27 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index a10cbaf..68a19bd 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -604,8 +604,33 @@ static int apply_one_level(struct domain *d,
> >              return P2M_ONE_PROGRESS_NOP;
> >          }
> >  
> > -        if ( level < 3 && p2m_table(orig_pte) )
> > -            return P2M_ONE_DESCEND;
> > +        if ( level < 3 )
> > +        {
> > +            if ( p2m_table(orig_pte) )
> > +                return P2M_ONE_DESCEND;
> > +
> > +            if ( op == REMOVE &&
> > +                 !is_mapping_aligned(*addr, end_gpaddr,
> > +                                     0, /* maddr doesn't matter for remove 
> > */
> > +                                     level_size) )
> > +            {
> > +                /*
> > +                 * Removing a mapping from the middle of a superpage. 
> > Shatter
> > +                 * and descend.
> > +                 */
> > +                *flush = true;
> > +                rc = p2m_create_table(d, entry,
> > +                                      level_shift - PAGE_SHIFT, 
> > flush_cache);
> > +                if ( rc < 0 )
> > +                    return rc;
> > +
> > +                p2m->stats.shattered[level]++;
> > +                p2m->stats.mappings[level]--;
> > +                p2m->stats.mappings[level+1] += LPAE_ENTRIES;
> > +
> > +                return P2M_ONE_DESCEND;
> > +            }
> > +        }
> 
> I would put the code below (which is only for level3) in the else part.
> This will also remove now useless "if ( level == 3 )".

This would break the relinquish case, I think.

Although thinking about it perhaps relinquish should behave like remove
and shatter. My original thinking was that we only ever relinquish the
entire address space so we shouldn't really be hitting that case.

Ian.


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