Re: [Xen-devel] [PATCH 04/18] arm/altp2m: Add altp2m init/teardown routines.

[Julien Grall <julien.grall@xxxxxxx>]

On 04/07/16 17:51, Sergej Proskurin wrote:
> On 07/04/2016 06:15 PM, Julien Grall wrote:
> > On 04/07/16 12:45, Sergej Proskurin wrote:
> > > +static void p2m_teardown_hostp2m(struct domain *d)
> > > +{
> > > +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > > +    struct page_info *pg = NULL;
> > > +    mfn_t mfn;
> > > +    unsigned int i;
> > > +
> > > +    spin_lock(&p2m->lock);
> > >
> > > -    if ( p2m->root )
> > > -        free_domheap_pages(p2m->root, P2M_ROOT_ORDER);
> > > +    while ( (pg = page_list_remove_head(&p2m->pages)) )
> > > +        if ( pg != p2m->root )
> > > +        {
> > > +            mfn = _mfn(page_to_mfn(pg));
> > > +            clear_domain_page(mfn);
> > Can you explain why you are cleaning the page here? It was not part of
> > p2m_teardown before this series.
> With the x86-based altp2m implementation, we experienced the problem
> that altp2m-teardowns did not clean the pages. As a result, later
> re-initialization reused the pages, which subsequently led to faults or
> crashes due to reused mappings. We additionally clean the altp2m pages
> and for the sake of completeness we clean the hostp2m tables as well.
All the pages allocated for the p2m are cleared before any usage (see 
p2m_create_table and p2m_allocate_table). So there is no point to zero 
the page here.
Also, unlike x86 we don't have a pool of page and directly use 
alloc_domheap_page to allocate a new page. So this would not prevent to 
get a non-zeroed page.
In any case, a such change should be in a separate patch and not hidden 
among big changes.
Regards,

--
Julien Grall

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