Re: [PATCH v2 10/25] arm/altp2m: Add HVMOP_altp2m_create_p2m.

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

Hello Sergej,

On 01/08/16 18:10, Sergej Proskurin wrote:
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
> v2: Cosmetic fixes.
> ---
>  xen/arch/arm/altp2m.c        | 23 +++++++++++++++++++++++
>  xen/arch/arm/hvm.c           |  3 ++-
>  xen/include/asm-arm/altp2m.h |  4 ++++
>  3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
> index e73424c..c22d2e4 100644
> --- a/xen/arch/arm/altp2m.c
> +++ b/xen/arch/arm/altp2m.c
> @@ -136,6 +136,29 @@ int altp2m_init_by_id(struct domain *d, unsigned int idx)
>      return rc;
>  }
>
> +int altp2m_init_next(struct domain *d, uint16_t *idx)
> +{
> +    int rc = -EINVAL;
> +    unsigned int i;
> +
> +    altp2m_lock(d);
> +
> +    for ( i = 0; i < MAX_ALTP2M; i++ )
> +    {
> +        if ( d->arch.altp2m_vttbr[i] != INVALID_VTTBR )
> +            continue;
> +
> +        rc = altp2m_init_helper(d, i);
> +        *idx = (uint16_t) i;

The cast is not necessary. You could make i uint16_t.

> +
> +        break;
> +    }
> +
> +    altp2m_unlock(d);
> +
> +    return rc;
> +}
> +
>  int altp2m_init(struct domain *d)
>  {
>      unsigned int i;
> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
> index 78370c6..063a06b 100644
> --- a/xen/arch/arm/hvm.c
> +++ b/xen/arch/arm/hvm.c
> @@ -120,7 +120,8 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>
>      case HVMOP_altp2m_create_p2m:
> -        rc = -EOPNOTSUPP;
> +        if ( !(rc = altp2m_init_next(d, &a.u.view.view)) )
> +            rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
>          break;
>
>      case HVMOP_altp2m_destroy_p2m:
> diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
> index 3ba82a8..3ecae27 100644
> --- a/xen/include/asm-arm/altp2m.h
> +++ b/xen/include/asm-arm/altp2m.h
> @@ -53,6 +53,10 @@ void altp2m_vcpu_destroy(struct vcpu *v);
>  int altp2m_init_by_id(struct domain *d,
>                        unsigned int idx);
>
> +/* Find an available alternate p2m and make it valid */

The comment and the implementation don't match the name of the function. 
I would rename the function altp2m_find_available or something similar.

> +int altp2m_init_next(struct domain *d,
> +                     uint16_t *idx);
> +
>  /* Flush all the alternate p2m's for a domain */
>  void altp2m_flush(struct domain *d);
>
>

Regards,

-- 
Julien Grall

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