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

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,


On 08/03/2016 08:02 PM, Julien Grall wrote:
> Hello Sergej,
>
> Title: s/altp2m/p2m/ and please remove the full stop.
>
> Also this is not really a cosmetic change.

Ok, thank you.

>
> 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.
>

Fair enough. I will change this in the next patch series.

>>  {
>> -    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.
>

Ok.

>>  {
>> -    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;
>>
>>
>

Best regards,
~Sergej

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