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

[Sergej Proskurin <proskurin@sec.in.tum.de>]

Hi Julien,


On 07/04/2016 08:34 PM, Julien Grall wrote:
> 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,
>

Thank you Julien for clearing that up, I will remove the part that
clears the page to be freed.

Best regards,
Sergej


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