[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 6/9] xen: arm: handle variable p2m levels in p2m_lookup
Hi Ian, On 07/30/2014 02:47 PM, Ian Campbell wrote: > This paves the way for boot-time selection of the number of levels to > use in the p2m, which is required to support both 40-bit and 48-bit > systems. For now the starting level remains a compile time constant. > > Implemented by turning the linear sequence of lookups into a loop. > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > --- > xen/arch/arm/p2m.c | 70 > +++++++++++++++++++++++++++++++++------------------- > 1 file changed, 45 insertions(+), 25 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 225d125..557663f 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -149,45 +149,69 @@ static lpae_t *p2m_map_first(struct p2m_domain *p2m, > paddr_t addr) > paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t) > { > struct p2m_domain *p2m = &d->arch.p2m; > - lpae_t pte, *first = NULL, *second = NULL, *third = NULL; > + const unsigned int offsets[4] = { > + zeroeth_table_offset(paddr), > + first_table_offset(paddr), > + second_table_offset(paddr), > + third_table_offset(paddr) > + }; > + const paddr_t masks[4] = { > + ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK > + }; > + lpae_t pte, *map; > paddr_t maddr = INVALID_PADDR; > paddr_t mask; > p2m_type_t _t; > + int level, root_table; I would use unsigned int here. > + > + BUILD_BUG_ON(THIRD_MASK != PAGE_MASK); > > /* Allow t to be NULL */ > t = t ?: &_t; > > *t = p2m_invalid; > > + if ( P2M_ROOT_PAGES > 1 ) > + { > + /* > + * Concatenated root-level tables. The table number will be > + * the offset at the previous level. It is not possible to > + * concetenate a level-0 root. concatenate > + */ > + BUG_ON(P2M_ROOT_LEVEL == 0); I don't think this check is necessary in a production build (ie debug=n). If people is porting a new board, then it should be done in debug mode. Hence it should be a bit "faster" as this code will be called often. I would switch this BUG_ON to ASSERT. > + root_table = offsets[P2M_ROOT_LEVEL - 1]; > + if ( root_table >= P2M_ROOT_PAGES ) > + goto err; > + } > + else > + root_table = 0; > + > spin_lock(&p2m->lock); > > - first = p2m_map_first(p2m, paddr); > - if ( !first ) > - goto err; > + map = __map_domain_page(p2m->root + root_table); > > - mask = FIRST_MASK; > - pte = first[first_table_offset(paddr)]; > - if ( !p2m_table(pte) ) > - goto done; > + for ( level = P2M_ROOT_LEVEL ; level < 4 ; level++ ) > + { > + mask = masks[level]; > > - mask = SECOND_MASK; > - second = map_domain_page(pte.p2m.base); > - pte = second[second_table_offset(paddr)]; > - if ( !p2m_table(pte) ) > - goto done; > + pte = map[offsets[level]]; > > - mask = THIRD_MASK; > + if ( level == 3 && !p2m_table(pte) ) > + /* Invalid, clobber the pte */ > + pte.bits = 0; > + if ( level == 3 || !p2m_table(pte) ) > + /* Done */ > + break; > > - BUILD_BUG_ON(THIRD_MASK != PAGE_MASK); > + BUG_ON(level == 3); Rather than checking level == 3, I would add BUG_ON(level < 4) at the end of loop. The only drawback to be there is we map one more page. And same remark as the previous BUG_ON(). Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |