Re: [Xen-devel] [PATCH v4 04/17] xen/arm: ITS: Add helper functions to manage its_devices

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Wed, Jul 15, 2015 at 4:07 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hi Vijay,
>
>
> On 10/07/2015 09:42, vijay.kilari@xxxxxxxxx wrote:
>>
>> ---
>>   xen/arch/arm/gic-v3-its.c     |   49
>> +++++++++++++++++++++++++++++++++++++++++
>>   xen/include/asm-arm/gic-its.h |   13 +++++++++++
>>   2 files changed, 62 insertions(+)
>>
>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> index 60ab646..b421a6f 100644
>> --- a/xen/arch/arm/gic-v3-its.c
>> +++ b/xen/arch/arm/gic-v3-its.c
>> @@ -90,6 +90,7 @@ struct its_node {
>>   static LIST_HEAD(its_nodes);
>>   static DEFINE_SPINLOCK(its_lock);
>>   static struct rdist_prop  *gic_rdists;
>> +static struct rb_root rb_its_dev;
>>
>>   #define gic_data_rdist()    (per_cpu(rdist, smp_processor_id()))
>>
>> @@ -101,6 +102,53 @@ void dump_cmd(its_cmd_block *cmd)
>>   }
>>   #endif
>>
>> +/* RB-tree helpers for its_device */
>> +struct its_device *its_find_device(u32 devid)
>> +{
>> +    struct rb_node *node = rb_its_dev.rb_node;
>> +
>> +    while ( node )
>> +    {
>> +        struct its_device *dev;
>> +
>> +        dev = container_of(node, struct its_device, node);
>> +        if ( devid < dev->device_id )
>> +            node = node->rb_left;
>> +        else if ( devid > dev->device_id )
>> +            node = node->rb_right;
>> +        else
>> +            return dev;
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +int its_insert_device(struct its_device *dev)
>> +{
>> +    struct rb_node **new, *parent;
>> +
>> +    new = &rb_its_dev.rb_node;
>> +    parent = NULL;
>> +    while ( *new )
>> +    {
>> +        struct its_device *this;
>> +
>> +        this  = container_of(*new, struct its_device, node);
>> +        parent = *new;
>> +        if ( dev->device_id < this->device_id )
>> +            new = &((*new)->rb_left);
>> +        else if ( dev->device_id > this->device_id )
>> +            new = &((*new)->rb_right);
>> +        else
>> +            return -EEXIST;
>> +    }
>> +
>> +    rb_link_node(&dev->node, parent, new);
>> +    rb_insert_color(&dev->node, &rb_its_dev);
>> +
>> +    return 0;
>> +}
>> +
>>   #define ITS_CMD_QUEUE_SZ            SZ_64K
>>   #define ITS_CMD_QUEUE_NR_ENTRIES    (ITS_CMD_QUEUE_SZ /
>> sizeof(its_cmd_block))
>>
>> @@ -811,6 +859,7 @@ static int its_probe(struct dt_device_node *node)
>>       list_add(&its->entry, &its_nodes);
>>       spin_unlock(&its_lock);
>>
>> +    rb_its_dev = RB_ROOT;
>
>
> NIT: missing newline here.
>
> A general question, how do you ensure any concurrent insert to the RB-tree?
> Is it taken care by the ITS lock and the caller?
>
> I would be ok to defer the locking for when the PCI passthrough is supported
> as all the ITS device are added when Xen is booted.
>
> A TODO would be worth to add. With the TODO and the answer to my questions:

I have added new lock "rb_its_dev_lock" in patch #8 where caller is managing.

>
> Reviewed-by: Julien Grall <julien.grall@xxxxxxxxxx>
>
> Regards,
>
> --
> Julien Grall

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