[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC v2 4/6] xen/arm: Implement VLPT for guest p2m mapping in live migration
Hi Wei, I will try to not repeat Andrew's comments. On 04/15/2014 10:05 PM, Wei Huang wrote: > xen/arch/arm/domain.c | 5 ++ > xen/arch/arm/mm.c | 116 > ++++++++++++++++++++++++++++++++++++++++++ I think the functions you've added in mm.c should be part of p2m.c. [..] > +/* Restore the xen page table for vlpt mapping for domain */ > +void restore_vlpt(struct domain *d) > +{ > + int i; > + > + dsb(sy); I think inner-shareable (ish) is enough here. > + > + for ( i = d->arch.dirty.second_lvl_start; i < > d->arch.dirty.second_lvl_end; > + ++i ) > + { > + int k = i % LPAE_ENTRIES; > + int l = i / LPAE_ENTRIES; > + > + if ( xen_second[i].bits != d->arch.dirty.second_lvl[l][k].bits ) > + { > + write_pte(&xen_second[i], d->arch.dirty.second_lvl[l][k]); > + flush_xen_data_tlb_range_va(i << SECOND_SHIFT, 1 << > SECOND_SHIFT); > + } > + } > + > + dsb(sy); Same here. > + isb(); > +} > + > +/* Set up the xen page table for vlpt mapping for domain */ > +int prepare_vlpt(struct domain *d) > +{ > + int xen_second_linear_base; > + int gp2m_start_index, gp2m_end_index; > + struct p2m_domain *p2m = &d->arch.p2m; > + struct page_info *second_lvl_page; > + paddr_t gma_start = 0; > + paddr_t gma_end = 0; > + lpae_t *first[2]; > + int i; > + uint64_t required, avail = VIRT_LIN_P2M_END - VIRT_LIN_P2M_START; > + > + domain_get_gpfn_range(d, &gma_start, &gma_end); > + required = (gma_end - gma_start) >> LPAE_SHIFT; > + > + if ( required > avail ) What is the limit of RAM? > + { > + dprintk(XENLOG_ERR, "Available VLPT is small for domU guest" > + "(avail: %llx, required: %llx)\n", (unsigned long long)avail, > + (unsigned long long)required); Why do you cast here? > + return -ENOMEM; > + } > + > + xen_second_linear_base = second_linear_offset(VIRT_LIN_P2M_START); > + > + gp2m_start_index = gma_start >> FIRST_SHIFT; > + gp2m_end_index = (gma_end >> FIRST_SHIFT) + 1; > + > + if ( xen_second_linear_base + gp2m_end_index >= LPAE_ENTRIES * 2 ) > + { In which case, this thing happen? > + dprintk(XENLOG_ERR, "xen second page is small for VLPT for domU"); > + return -ENOMEM; > + } > + > + second_lvl_page = alloc_domheap_pages(NULL, 1, 0); > + if ( second_lvl_page == NULL ) > + return -ENOMEM; > + > + /* First level p2m is 2 consecutive pages */ > + d->arch.dirty.second_lvl[0] = map_domain_page_global( > + page_to_mfn(second_lvl_page) ); > + d->arch.dirty.second_lvl[1] = map_domain_page_global( > + page_to_mfn(second_lvl_page+1) ); map_domain_page_global can fail. > + > + first[0] = __map_domain_page(p2m->first_level); > + first[1] = __map_domain_page(p2m->first_level+1); > + > + for ( i = gp2m_start_index; i < gp2m_end_index; ++i ) > + { > + int k = i % LPAE_ENTRIES; > + int l = i / LPAE_ENTRIES; > + int k2 = (xen_second_linear_base + i) % LPAE_ENTRIES; > + int l2 = (xen_second_linear_base + i) / LPAE_ENTRIES; > + > + write_pte(&xen_second[xen_second_linear_base+i], first[l][k]); spaces should surrounding binary operator. I think you can create a temporary variable to store first[l][k]. It will avoid two load. > + > + /* we copy the mapping into domain's structure as a reference > + * in case of the context switch (used in restore_vlpt) */ > + d->arch.dirty.second_lvl[l2][k2] = first[l][k]; > + } > + unmap_domain_page(first[0]); > + unmap_domain_page(first[1]); > + > + /* storing the start and end index */ > + d->arch.dirty.second_lvl_start = xen_second_linear_base + > gp2m_start_index; > + d->arch.dirty.second_lvl_end = xen_second_linear_base + gp2m_end_index; > + > + flush_vlpt(d); > + > + return 0; > +} > + > +void cleanup_vlpt(struct domain *d) > +{ > + /* First level p2m is 2 consecutive pages */ > + unmap_domain_page_global(d->arch.dirty.second_lvl[0]); > + unmap_domain_page_global(d->arch.dirty.second_lvl[1]); > +} Newline here please. > /* Fixmap slots */ > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index 28c359a..5321bd6 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -161,6 +161,13 @@ struct arch_domain > spinlock_t lock; > } vuart; > > + /* dirty-page tracing */ > + struct { > + volatile int second_lvl_start; /* for context switch */ > + volatile int second_lvl_end; Can you please comment theses 2 fields. What does it mean? > + lpae_t *second_lvl[2]; /* copy of guest p2m's first */ > + } dirty; > + > unsigned int evtchn_irq; > } __cacheline_aligned; > > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > index 8347524..5fd684f 100644 > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -4,6 +4,7 @@ > #include <xen/config.h> > #include <xen/kernel.h> > #include <asm/page.h> > +#include <asm/config.h> > #include <public/xen.h> > > /* Align Xen to a 2 MiB boundary. */ > @@ -342,6 +343,22 @@ static inline void put_page_and_type(struct page_info > *page) > put_page(page); > } > > +int prepare_vlpt(struct domain *d); > +void cleanup_vlpt(struct domain *d); > +void restore_vlpt(struct domain *d); > + > +/* calculate the xen's virtual address for accessing the leaf PTE of > + * a given address (GPA) */ > +static inline lpae_t * get_vlpt_3lvl_pte(paddr_t addr) No space between * and the function name. > +{ > + lpae_t *table = (lpae_t *)VIRT_LIN_P2M_START; > + > + /* Since we slotted the guest's first p2m page table to xen's > + * second page table, one shift is enough for calculating the > + * index of guest p2m table entry */ > + return &table[addr >> PAGE_SHIFT]; > +} > + 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 |