Re: [Xen-devel] [PATCH RESEND v9 01/14] arch/arm: add consistency check to REMOVE p2m changes

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Mon, 2014-07-14 at 11:57 +0100, Julien Grall wrote:
> Hi Arianna,
> 
> On 14/07/14 01:50, Arianna Avanzini wrote:
> >   xen/arch/arm/p2m.c | 21 ++++++++++++++++++++-
> >   1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index c0c011a..5a77189 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -596,12 +596,30 @@ static int apply_one_level(struct domain *d,
> >
> >           break;
> >
> > -    case RELINQUISH:
> >       case REMOVE:
> > +        if ( is_mapping_aligned(*addr, end_gpaddr, *maddr, level_size) )
> > +        {
> > +            unsigned long mfn = orig_pte.p2m.base;
> 
> I think the check is misplaced, it should go after:
> 
> if ( level < 3 && p2m_table(orig_pte) )
>    return P2M_ONE_DESCEND;

Correct.

Note that this also follows a check for non-present entries. so that
part of the check can be dropped as well.

> 
> It's because this code path can now be called with either a P2M Table or 
> a P2M entry. You will get wrong error message and may log many times the 
> same issue.
> 
> Also I've reported an issue to Ian about this code path today. If the 
> guest balloning driver or your MMIO unmap a single 4K entry, it may fail 
> or remove a  superpage by mistake.

Right, see my patch "xen: arm: correctly handle removing a subset of a
superpage mapping." from Monday.

Ian.


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