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

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Tue, Feb 24, 2015 at 6:29 PM, Julien Grall <julien.grall@xxxxxxxxxx> 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?

MAP_SMALL_PAGES is already defined as WRITE_ALLOC which occupies lower 3 bits
what is need for PAGE_AVAIL0?

Below definitions should suffice?

-#define PTE_INVALID   (0x1 << 16)
+#define PTE_PRESENT   (0x1 << 16)

-#define PAGE_HYPERVISOR         (WRITEALLOC)
-#define PAGE_HYPERVISOR_NOCACHE (DEV_SHARED)
-#define PAGE_HYPERVISOR_WC      (DEV_WC)
-#define MAP_SMALL_PAGES         (PAGE_HYPERVISOR | (PTE_INVALID))
+#define PAGE_HYPERVISOR         ((WRITEALLOC) | PTE_PRESENT )
+#define PAGE_HYPERVISOR_NOCACHE ((DEV_SHARED) | PTE_PRESENT )
+#define PAGE_HYPERVISOR_WC      ((DEV_WC) | PTE_PRESENT )
+#define MAP_SMALL_PAGES         (WRITEALLOC)
+
+#define is_pte_present(x) ((x) & PTE_PRESENT)
+#define get_pte_flags(x)  ((x) & 0x7)


>
> 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

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