Re: [PATCH RFC 4/6] xen/arm: mm: Allow other mapping size in xen_pt_update_entry()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 20 Nov 2020, Julien Grall wrote:
> > >       /*
> > >        * For arm32, page-tables are different on each CPUs. Yet, they
> > > share
> > > @@ -1265,14 +1287,43 @@ static int xen_pt_update(unsigned long virt,
> > >         spin_lock(&xen_pt_lock);
> > >   -    for ( ; addr < addr_end; addr += PAGE_SIZE )
> > > +    while ( left )
> > >       {
> > > -        rc = xen_pt_update_entry(root, addr, mfn, flags);
> > > +        unsigned int order;
> > > +        unsigned long mask;
> > > +
> > > +        /*
> > > +         * Don't take into account the MFN when removing mapping (i.e
> > > +         * MFN_INVALID) to calculate the correct target order.
> > > +         *
> > > +         * XXX: Support superpage mappings if nr is not aligned to a
> > > +         * superpage size.
> > 
> > It would be good to add another sentence to explain that the checks
> > below are simply based on masks and rely on the mfn, vfn, and also
> > nr_mfn to be superpage aligned. (It took me some time to figure it out.)
> 
> I am not sure to understand what you wrote here. Could you suggest a sentence?

Something like the following:

/*
 * Don't take into account the MFN when removing mapping (i.e
 * MFN_INVALID) to calculate the correct target order.
 *
 * This loop relies on mfn, vfn, and nr_mfn, to be all superpage
 * aligned, and it uses `mask' to check for that.
 *
 * XXX: Support superpage mappings if nr_mfn is not aligned to a
 * superpage size.
 */


> Regarding the TODO itself, we have the exact same one in the P2M code. I
> couldn't find a clever way to deal with it yet. Any idea how this could be
> solved?
 
I was thinking of a loop that start with the highest possible superpage
size that virt and mfn are aligned to, and also smaller or equal to
nr_mfn. So rather than using the mask to also make sure nr_mfns is
aligned, I would only use the mask to check that mfn and virt are
aligned. Then, we only need to check that superpage_size <= left.

Concrete example: virt and mfn are 2MB aligned, nr_mfn is 5MB / 1280 4K
pages. We allocate 2MB superpages until onlt 1MB is left. At that point
superpage_size <= left fails and we go down to 4K allocations.

Would that work?