Re: [Xen-devel] [PATCH v2 4/6] xen/arm: implement get/put_page_type

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 23 Jul 2012, Ian Campbell wrote:
> On Fri, 2012-07-20 at 16:05 +0100, Stefano Stabellini wrote:
> > Add a basic get_page_type and put_page_type implementation: the
> > implementation is similar to the x86 one, without all the code to handle
> > shadow pagetables and other unneeded features.
> > 
> > Also remove PGT_shared_page, that is unused.
> 
> I suppose it'll come back some day?

Only if we want to: it is not needed by common code.


> Is the existing page sharing stuff completely contained within the x86
> arch code and not in generic code at all?

Yes.


> > +int get_page_type(struct page_info *page, unsigned long type)
> > +{
> > +    unsigned long nx, x, y = page->u.inuse.type_info;
> > +
> > +    for ( ; ; )
> > +    {
> > +        x  = y;
> > +        nx = x + 1;
> > +        if ( unlikely((nx & PGT_count_mask) == 0) )
> > +        {
> > +            printk("Type count overflow on pfn %lx", page_to_mfn(page));
> > +            return -EINVAL;
> > +        }
> > +        else if ( (x & PGT_count_mask) == 0 )
> > +        {
> > +            struct domain *d = page_get_owner(page);
> > +
> > +            if ( (x & PGT_type_mask) != type )
> > +            {
> > +                /*
> > +                 * On type change we check to flush stale TLB entries. 
> > This 
> > +                 * may be unnecessary (e.g., page was GDT/LDT) but those 
> > +                 * circumstances should be very rare.
> > +                 */
> > +                cpumask_t mask;
> > +
> > +                cpumask_copy(&mask, d->domain_dirty_cpumask);
> > +
> > +                /* Don't flush if the timestamp is old enough */
> > +                tlbflush_filter(mask, page->tlbflush_timestamp);
> > +
> > +                if ( unlikely(!cpumask_empty(&mask)) )
> > +                {
> > +                    perfc_incr(need_flush_tlb_flush);
> > +                    flush_tlb_mask(&mask);
> > +                }
> > +
> > +                /* We lose existing type and validity. */
> > +                nx &= ~(PGT_type_mask | PGT_validated);
> > +                nx |= type;
> > +
> > +                /* No special validation needed for writable pages. */
> > +                /* Page tables and GDT/LDT need to be scanned for 
> > validity. */
> 
> There is no GDT/LDT on ARM.
> 
> And do we really need PGT_validated when there is no direct paging
> support? (Hrm, maybe I'm confused about what PGT_validated means).
> 
> In fact, do the types make any sense at all for ARM? Is PGT_writable is
> really only meaningful when you have direct paging?

I am starting to think that a better way to implement get_page_type is:

int get_page_type(struct page_info *page, unsigned long type)
{
    return 1;
}

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