Re: [PATCH v1] xen/arm: Do not allocate pte entries for MAP_SMALL_PAGES

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2015-02-24 at 12:59 +0000, Julien Grall wrote:
> On 24/02/15 10:26, Ian Campbell wrote:
> > On Tue, 2015-02-24 at 09:38 +0000, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 24/02/2015 09:31, Ian Campbell wrote:
> >>> On Wed, 2015-02-18 at 13:03 +0000, Julien Grall wrote:
> >>>>> +                {
> >>>>> +                    pte = mfn_to_xen_entry(mfn, (ai & 0xffff));
> >>>>
> >>>> Please introduce a new macro for the mask.
> >>>
> >>> Better would be a pte_foo accessor, similar (if not identical) to x86's
> >>> pte_get_flags. So pte_get_flags(ai) or so.
> >>
> >> I'm not able to find a such function in x86. Did you intend to mean 
> >> pte_flags_to_cacheattr?
> > 
> > It's actually get_pte_flags.
> > 
> >> In another side, using PTE_PRESENT would require to introduce a 
> >> PAGE_AVAIL0 (or smth similar).
> > 
> > Why?
> 
> If we have only a bit PTE_PRESENT, how do you define MAP_SMALL_PAGES?

Oh right, yes, I was thinking that would use some other bit, either an
avail bit or reusing the table bit or something.

> Anyway, I guess introduce a separate helper would help here.
> 
> >>> #define PTE_PRESENT ((struct lpae_t){ .pt.present = 1 }).bits
> >>>
> >>> probably doesn't work, I'm not even sure if this sort of thing is
> >>> possible. If not then "#define PTE_PRESET (1ULL<<0)".
> >>
> >> The attribute index (write-alloc, buferrable...) is using the less 
> >> significant 3 bits. So I was suggesting to use the top of the word.
> > 
> > I was suggesting to use bits 2..4 as in the real PTE, to be more similar
> > to the x86 interpretation of this argument.
> 
> I don't think we have to follow how x86 interpret this argument. This is
> just a series of flags and may or may not match a bit in the PTE.

Not matching x86 here has already led to one set of confusion.

I'm not saying with have to match x86, but we should strongly consider
it and not just run with what we have now because it is a smaller
change.

Ian.