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

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,


On 08/03/2016 08:48 PM, Julien Grall wrote:
> Hello Sergej,
>
> On 01/08/16 18:10, Sergej Proskurin wrote:
>> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
>> ---
>> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Cc: Julien Grall <julien.grall@xxxxxxx>
>> ---
>> 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.
>

Ok.

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

Ok, thank you.

>> +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);
>>
>>
>

Best regards,
~Sergej

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