Re: [PATCH v3 15/38] arm/p2m: Add altp2m table flushing routine

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

Hello Sergej,

On 16/08/16 23:16, Sergej Proskurin wrote:
> The current implementation differentiates between flushing and
> destroying altp2m views. This commit adds the function altp2m_flush,
> which allows to release all of the alternate p2m views.
>
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
> v2: Pages in p2m->pages are not cleared in p2m_flush_table anymore.
>     VMID is freed in p2m_free_one.
>     Cosmetic fixes.
>
> v3: Changed the locking mechanism to "p2m_write_lock" inside the
>     function "altp2m_flush".
>
>     Do not flush but rather teardown the altp2m in the function
>     "altp2m_flush".
>
>     Exchanged the check "altp2m_vttbr[idx] == INVALID_VTTBR" for
>     "altp2m_p2m[idx] == NULL" in "altp2m_flush".
> ---
>  xen/arch/arm/altp2m.c        | 31 +++++++++++++++++++++++++++++++
>  xen/include/asm-arm/altp2m.h |  3 +++
>  2 files changed, 34 insertions(+)
>
> diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
> index 66a373a..02cffd7 100644
> --- a/xen/arch/arm/altp2m.c
> +++ b/xen/arch/arm/altp2m.c
> @@ -34,6 +34,37 @@ int altp2m_init(struct domain *d)
>      return 0;
>  }
>
> +void altp2m_flush(struct domain *d)
> +{
> +    unsigned int i;
> +    struct p2m_domain *p2m;
> +
> +    /*
> +     * If altp2m is active, we are not allowed to flush altp2m[0]. This special
> +     * view is considered as the hostp2m as long as altp2m is active.
> +     */
> +    ASSERT(!altp2m_active(d));
> +
> +    altp2m_lock(d);
> +
> +    for ( i = 0; i < MAX_ALTP2M; i++ )
> +    {
> +        if ( d->arch.altp2m_p2m[i] == NULL )
> +            continue;
> +
> +        p2m = d->arch.altp2m_p2m[i];
> +
> +        p2m_write_lock(p2m);

Why do you take the lock here? The p2m should not be used by anyone at 
that time.

> +        p2m_teardown_one(p2m);

You may want to add an ASSERT(!atomic_read(p2m->active_vcpus)) somewhere.

> +        p2m_write_unlock(p2m);
> +
> +        xfree(p2m);
> +        d->arch.altp2m_p2m[i] = NULL;
> +    }
> +
> +    altp2m_unlock(d);
> +}
> +
>  void altp2m_teardown(struct domain *d)
>  {
>      unsigned int i;
> diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
> index a156109..4c15b75 100644
> --- a/xen/include/asm-arm/altp2m.h
> +++ b/xen/include/asm-arm/altp2m.h
> @@ -42,4 +42,7 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
>  int altp2m_init(struct domain *d);
>  void altp2m_teardown(struct domain *d);
>
> +/* Flush all the alternate p2m's for a domain. */
> +void altp2m_flush(struct domain *d);
> +
>  #endif /* __ASM_ARM_ALTP2M_H */
>

Regards,

-- 
Julien Grall

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