Re: [PATCH 06/18] arm/altp2m: Add a(p2m) table flushing routines.

[Julien Grall <julien.grall@arm.com>]

Hello Sergej,

On 04/07/16 12:45, Sergej Proskurin wrote:
> The current implementation differentiates between flushing and
> destroying altp2m views. This commit adds the functions
> p2m_flush_altp2m, and p2m_flush_table, which allow to flush all or
> individual altp2m views without destroying the entire table. In this
> way, altp2m views can be reused at a later point in time.
>
> In addition, the implementation clears all altp2m entries during the
> process of flushing. The same applies to hostp2m entries, when it is
> destroyed. In this way, further domain and p2m allocations will not
> unintentionally reuse old p2m mappings.
>
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
>   xen/arch/arm/p2m.c        | 67 +++++++++++++++++++++++++++++++++++++++++++++++
>   xen/include/asm-arm/p2m.h | 15 ++++++++---
>   2 files changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 4a745fd..ae789e6 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -2110,6 +2110,73 @@ int p2m_init_altp2m_by_id(struct domain *d, unsigned int idx)
>       return rc;
>   }
>
> +/* Reset this p2m table to be empty */
> +static void p2m_flush_table(struct p2m_domain *p2m)
> +{
> +    struct page_info *top, *pg;
> +    mfn_t mfn;
> +    unsigned int i;
> +
> +    /* Check whether the p2m table has already been flushed before. */
> +    if ( p2m->root == NULL)

This check looks invalid. p2m->root is never reset to NULL by 
p2m_flush_table, so you will always flush.

> +        return;
> +
> +    spin_lock(&p2m->lock);
> +
> +    /*
> +     * "Host" p2m tables can have shared entries &c that need a bit more care
> +     * when discarding them

I don't understand this comment. Can you explain it?

> +     */
> +    ASSERT(!p2m_is_hostp2m(p2m));
> +
> +    /* Zap the top level of the trie */
> +    top = p2m->root;
> +
> +    /* Clear all concatenated first level pages */
> +    for ( i = 0; i < P2M_ROOT_PAGES; i++ )
> +    {
> +        mfn = _mfn(page_to_mfn(top + i));
> +        clear_domain_page(mfn);
> +    }
> +
> +    /* Free the rest of the trie pages back to the paging pool */
> +    while ( (pg = page_list_remove_head(&p2m->pages)) )
> +        if ( pg != top  )
> +        {
> +            /*
> +             * Before freeing the individual pages, we clear them to prevent
> +             * reusing old table entries in future p2m allocations.
> +             */
> +            mfn = _mfn(page_to_mfn(pg));
> +            clear_domain_page(mfn);
> +            free_domheap_page(pg);
> +        }
> +
> +    page_list_add(top, &p2m->pages);

This code is very similar to p2m_free_one. Can we share some code?

> +
> +    /* Invalidate VTTBR */
> +    p2m->vttbr.vttbr = 0;
> +    p2m->vttbr.vttbr_baddr = INVALID_MFN;
> +
> +    spin_unlock(&p2m->lock);
> +}

Regards,

-- 
Julien Grall

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