Re: [Xen-devel] [PATCH v2 06/25] arm/altp2m: Cosmetic fixes - function prototypes.

[Julien Grall <julien.grall@xxxxxxx>]

Hello Sergej,

Title: s/altp2m/p2m/ and please remove the full stop.

Also this is not really a cosmetic change.

On 01/08/16 18:10, Sergej Proskurin wrote:
> This commit changes the prototype of the following functions:
> - p2m_alloc_vmid
> - p2m_free_vmid
>
> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/p2m.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 63fe3d9..ff9c0d1 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1337,11 +1337,10 @@ void p2m_vmid_allocator_init(void)
>      set_bit(INVALID_VMID, vmid_mask);
>  }
>
> -static int p2m_alloc_vmid(struct domain *d)
> +static int p2m_alloc_vmid(struct p2m_domain *p2m)
I am not a big fan of this interface change. I would much prefer if the 
VMID was returned and set to p2m->vmid by the caller.
So this would return either the VMID or INVALID_VMID.

>  {
> -    struct p2m_domain *p2m = &d->arch.p2m;
> -
>      int rc, nr;
> +    struct domain *d = p2m->domain;
>
>      spin_lock(&vmid_alloc_lock);
>
> @@ -1367,9 +1366,8 @@ out:
>      return rc;
>  }
>
> -static void p2m_free_vmid(struct domain *d)
> +static void p2m_free_vmid(struct p2m_domain *p2m)
Likewise here. It would be better if the function is taking a vmid in 
parameter.
>  {
> -    struct p2m_domain *p2m = &d->arch.p2m;
>      spin_lock(&vmid_alloc_lock);
>      if ( p2m->vmid != INVALID_VMID )
>          clear_bit(p2m->vmid, vmid_mask);
> @@ -1399,7 +1397,7 @@ static inline void p2m_free_one(struct p2m_domain *p2m)
>      p2m_flush_table(p2m);
>
>      /* Free VMID and reset VTTBR */
> -    p2m_free_vmid(p2m->domain);
> +    p2m_free_vmid(p2m);
>      p2m->vttbr.vttbr = INVALID_VTTBR;
>
>      if ( p2m->root )
> @@ -1417,7 +1415,7 @@ int p2m_init_one(struct domain *d, struct p2m_domain *p2m)
>      rwlock_init(&p2m->lock);
>      INIT_PAGE_LIST_HEAD(&p2m->pages);
>
> -    rc = p2m_alloc_vmid(d);
> +    rc = p2m_alloc_vmid(p2m);
>      if ( rc != 0 )
>          return rc;
Regards,

--
Julien Grall

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