From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v1] xen/arm: Do not allocate pte entries for MAP_SMALL_PAGES Date: Wed, 18 Feb 2015 13:03:39 +0000 Message-ID: <54E48DAB.5030307@linaro.org> References: <1424264213-30614-1-git-send-email-vijay.kilari@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1424264213-30614-1-git-send-email-vijay.kilari@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: vijay.kilari@gmail.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, stefano.stabellini@citrix.com, tim@xen.org, xen-devel@lists.xen.org Cc: Prasun.Kapoor@caviumnetworks.com, vijaya.kumar@caviumnetworks.com, manish.jaggi@caviumnetworks.com List-Id: xen-devel@lists.xenproject.org Hello Vijay, On 18/02/2015 12:56, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K > > On x86, for the pages mapped with PAGE_HYPERVISOR attribute > non-leaf page tables are allocated with valid pte entries. > and with MAP_SMALL_PAGES attribute only non-leaf page tables are > allocated with invalid (valid bit set to 0) pte entries. > However on arm this is not the case. On arm for the pages > mapped with PAGE_HYPERVISOR and MAP_SMALL_PAGES both > non-leaf and leaf level page table are allocated with valid bit > in pte entries. > > This behaviour in arm makes common vmap code fail to > allocate memory beyond 128MB as described below. > > In vmap_init, map_pages_to_xen() is called for mapping > vm_bitmap. Initially one page of vm_bitmap is allocated > and mapped using PAGE_HYPERVISOR attribute. > For the rest of vm_bitmap pages, MAP_SMALL_PAGES attribute > is used to map. > > In ARM for both PAGE_HYPERVISOR and MAP_SMALL_PAGES, valid bit > is set to 1 in pte entry for these mapping. > > In vma_alloc(), map_pages_to_xen() is failing for >128MB because > for this next vm_bitmap page the mapping is already set in vm_init() > with valid bit set in pte entry. So map_pages_to_xen() in > ARM returns error. > > With this patch, MAP_SMALL_PAGES attribute will only allocate > non-leaf page tables only. > > Here we use bit[16] in the attribute flag to know if leaf page > tables should be allocated or not. > > This bit is set only for MAP_SMALL_PAGES attribute. > > Signed-off-by: Vijaya Kumar K > --- > xen/arch/arm/mm.c | 9 ++++++--- > xen/include/asm-arm/page.h | 8 +++++++- > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 7d4ba0c..a12f3f5 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -865,9 +865,12 @@ static int create_xen_entries(enum xenmap_operation op, > addr, mfn); > return -EINVAL; > } > - pte = mfn_to_xen_entry(mfn, ai); > - pte.pt.table = 1; > - write_pte(&third[third_table_offset(addr)], pte); > + if ( !(ai & PTE_INVALID) ) you could do if ( ai & PTE_INVALID ) break; It would avoid a new level of indentation. Also, I would rename ai to flags as the variable is not anymore an attribute index. > + { > + pte = mfn_to_xen_entry(mfn, (ai & 0xffff)); Please introduce a new macro for the mask. > + pte.pt.table = 1; > + write_pte(&third[third_table_offset(addr)], pte); > + } > break; > case REMOVE: > if ( !third[third_table_offset(addr)].pt.valid ) > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h > index 3e7b0ae..80415b3 100644 > --- a/xen/include/asm-arm/page.h > +++ b/xen/include/asm-arm/page.h > @@ -61,10 +61,16 @@ > #define DEV_WC BUFFERABLE > #define DEV_CACHED WRITEBACK > > +/* bit 16 in the Attribute index can be used to know if > + * PTE entry should be added or not. This is useful > + * when ONLY non-leaf page table entries need to allocated > + */ > +#define PTE_INVALID (0x1 << 16) It makes more sense to introduce a PTE_PRESENT flags compare to PTE_INVALID. The former has more meaning that the latter. Regards, -- Julien Grall