Re: [Xen-devel] [PATCH V2 3/4] arm: allocate per-PCPU domheap pagetable pages

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-04-23 at 11:48 +0100, Tim Deegan wrote:

> s/PCPUS/PCPUs/

Ack to all the various typos...

> > + */
> > +
> > +/* Per-CPU pagetable pages */
> > +/* xen_pgtable == root of the trie (zeroeth level on 64-bit, first on 
> > 32-bit) */
> > +static DEFINE_PER_CPU(lpae_t *, xen_pgtable);
> > +/* xen_dommap == pages used by map_domain_page, these pages contain
> > + * the second level pagetables which mapp the domheap region
> > + * DOMHEAP_VIRT_START...DOMHEAP_VIRT_END in 2MB chunks. */
> > +static DEFINE_PER_CPU(lpae_t *, xen_dommap);
> > +
> > +/* Common pagetable leaves */
> > +/* Second level page tables.
> > + *
> > + * The second-level table is 2 contiguous pages long, and covers all
> > + * addresses from 0 to 0x7fffffff.
> 
> Can we keep the comment about using second_linear_offset to access
> xen_second?

Yes,sorry I rewrote this a few times and must have dropped it.

> > +    flush_xen_dcache_va_range(root, PAGE_SIZE);
> > +    flush_xen_dcache_va_range(domheap, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
> 
> Don't we need to flush 'first' as well?

Yes.

> > +    uint64_t ttbr;
> > +
> > +    /* Change to this CPUs pagetables */
> > +    flush_xen_text_tlb();
> > +
> > +    ttbr = (uintptr_t) virt_to_maddr(this_cpu(xen_pgtable));
> > +    WRITE_SYSREG64(ttbr, TTBR0_EL2);
> 
> isb() here, to make sure the CP write completes? 

Hrm, we don't have one in the primary CPU bring up either. 

flush_xen_text_tlb starts with an isb, which I suspect is sufficient? (I
vaguely remember having a conversation along those lines when this code
was changed to look like this).

> > +    dsb();                         /* Ensure visibility of HTTBR update */

I think this comment is actually misleading since it doesn't actually
ensure the visibility, just that things won't cross the boundary.

I'll try and improve the comment here and in the other site too, does
ths sound right?:
        WRITE_SYSREG64(ttbr, TTBR0_EL2);
        dsb(); /* ensure memory accesses do not cross over the TTBR0 write */
        /* flush_xen_text_tlb contains an ISB which ensures the
        TTBR0_EL2 
         * update has completed. */
        flush_xen_text_tlb();
?



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