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

[Julien Grall <julien.grall@linaro.org>]

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?

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.

For instance, in the case of MAP_SMALL_PAGES, we don't have to write the
final PTE.

Regards,

-- 
Julien Grall