Re: [lng-odp] [API-NEXT PATCH 13/21] linux-gen: drv: devio registration

[Christophe Milard]

On 22 February 2017 at 23:54, Bill Fischofer  wrote:
>
>
> On Wed, Feb 22, 2017 at 6:55 AM, Christophe Milard
>  wrote:
>>
>> devios (dev IO) provide a interface for drivers to access a device:
>> Devices enumerated by enumerators may be accessed in by different
>> mechanisms (depending on iommu presence or other factors). This extra
>> abstraction is provided by devios, which provide a sets of methods to
>> access the devices of a given type (i.e. registred enumerator(s)
>> enumerating devices of the same kind (e.g. PCI)).
>> This patch just implements the devio registration method provided by the
>> driver API.
>>
>> Signed-off-by: Christophe Milard 
>> ---
>>  platform/linux-generic/drv_driver.c | 134
>> +++-
>>  1 file changed, 131 insertions(+), 3 deletions(-)
>>
>> diff --git a/platform/linux-generic/drv_driver.c
>> b/platform/linux-generic/drv_driver.c
>> index 517a3c6..eb0dc48 100644
>> --- a/platform/linux-generic/drv_driver.c
>> +++ b/platform/linux-generic/drv_driver.c
>> @@ -28,6 +28,7 @@ static _odp_ishm_pool_t *list_elt_pool;
>>  typedef struct _odpdrv_enumr_class_s _odpdrv_enumr_class_t;
>>  typedef struct _odpdrv_enumr_s _odpdrv_enumr_t;
>>  typedef struct _odpdrv_device_s _odpdrv_device_t;
>> +typedef struct _odpdrv_devio_s _odpdrv_devio_t;
>>
>>  /* an enumerator class (list element) */
>>  struct _odpdrv_enumr_class_s {
>> @@ -72,6 +73,20 @@ typedef struct _odpdrv_device_lst_t {
>>  } _odpdrv_device_lst_t;
>>  static struct _odpdrv_device_lst_t device_lst;
>>
>> +/* a devio (list element) */
>> +struct _odpdrv_devio_s {
>> +   odpdrv_devio_param_t param;
>> +   _odp_ishm_pool_t *pool;
>> +   struct _odpdrv_devio_s *next;
>> +} _odpdrv_devio_s;
>> +
>> +/* the devio list: */
>> +typedef struct _odpdrv_devio_lst_t {
>> +   odp_rwlock_recursive_t lock;
>> +   _odpdrv_devio_t *head;
>> +} _odpdrv_devio_lst_t;
>> +static struct _odpdrv_devio_lst_t devio_lst;
>> +
>>  /* some driver elements (such as enumeraor classes, drivers, devio) may
>>   * register before init_global and init_local complete. Mutex will fail
>>   * in this cases but should be used later on.
>> @@ -149,6 +164,30 @@ static void dev_list_write_unlock(void)
>> odp_rwlock_recursive_write_unlock(_lst.lock);
>>  }
>>
>> +static void devio_list_read_lock(void)
>> +{
>> +   if (init_global_status == DONE)
>
>
> Same comment on the need for these guards.

same answer :-)

>
>>
>> +   odp_rwlock_recursive_read_lock(_lst.lock);
>> +}
>> +
>> +static void devio_list_read_unlock(void)
>> +{
>> +   if (init_global_status == DONE)
>> +   odp_rwlock_recursive_read_unlock(_lst.lock);
>> +}
>> +
>> +static void devio_list_write_lock(void)
>> +{
>> +   if (init_global_status == DONE)
>> +   odp_rwlock_recursive_write_lock(_lst.lock);
>> +}
>> +
>> +static void devio_list_write_unlock(void)
>> +{
>> +   if (init_global_status == DONE)
>> +   odp_rwlock_recursive_write_unlock(_lst.lock);
>> +}
>> +
>>  odpdrv_enumr_class_t
>> odpdrv_enumr_class_register(odpdrv_enumr_class_param_t
>>  *param)
>>  {
>> @@ -415,10 +454,65 @@ odpdrv_device_t *odpdrv_device_query(odpdrv_enumr_t
>> enumr, const char *address)
>>
>>  odpdrv_devio_t odpdrv_devio_register(odpdrv_devio_param_t *param)
>>  {
>> -   ODP_ERR("NOT Supported yet! Driver %s Registration!\n.",
>> -   param->api_name);
>> +   _odpdrv_devio_t *devio;
>> +
>> +   /* parse the list of already registered devios to make
>> +* sure no devio providing the same interface using th esame
>> enumerator
>> +* already exists:
>> +*/
>> +   devio_list_read_lock();
>> +   devio = devio_lst.head;
>
>
> Same comment on needing to initialize the head field (should be done at init
> time along with the lock).

same answer :-)

>
>>
>> +   while (devio) {
>> +   if ((strncmp(param->api_name, devio->param.api_name,
>> +ODPDRV_NAME_SIZE) == 0) &&
>> +   (strncmp(param->enumr_api_name,
>> devio->param.enumr_api_name,
>> +ODPDRV_NAME_SIZE) == 0)) {
>> +   ODP_ERR("a devio providing interface '%s' for
>> devices "
>> +   "of type '%s' is already registered\n!",
>> +   param->api_name, param->enumr_api_name);
>> +   devio_list_read_unlock();
>> +   return ODPDRV_DEVIO_INVALID;
>> +   }
>> +   devio = devio->next;
>> +   }
>> +   devio_list_read_unlock();
>>
>> -   return ODPDRV_DEVIO_INVALID;
>> +   /* allocate memory for the new devio:
>> +* If init_global has not been done yet, then, we cannot allocate
>> +* from any _ishm pool (ishm has not 

Re: [lng-odp] [API-NEXT PATCH 13/21] linux-gen: drv: devio registration

[Bill Fischofer]

On Wed, Feb 22, 2017 at 6:55 AM, Christophe Milard <
christophe.mil...@linaro.org> wrote:

> devios (dev IO) provide a interface for drivers to access a device:
> Devices enumerated by enumerators may be accessed in by different
> mechanisms (depending on iommu presence or other factors). This extra
> abstraction is provided by devios, which provide a sets of methods to
> access the devices of a given type (i.e. registred enumerator(s)
> enumerating devices of the same kind (e.g. PCI)).
> This patch just implements the devio registration method provided by the
> driver API.
>
> Signed-off-by: Christophe Milard 
> ---
>  platform/linux-generic/drv_driver.c | 134 ++
> +-
>  1 file changed, 131 insertions(+), 3 deletions(-)
>
> diff --git a/platform/linux-generic/drv_driver.c
> b/platform/linux-generic/drv_driver.c
> index 517a3c6..eb0dc48 100644
> --- a/platform/linux-generic/drv_driver.c
> +++ b/platform/linux-generic/drv_driver.c
> @@ -28,6 +28,7 @@ static _odp_ishm_pool_t *list_elt_pool;
>  typedef struct _odpdrv_enumr_class_s _odpdrv_enumr_class_t;
>  typedef struct _odpdrv_enumr_s _odpdrv_enumr_t;
>  typedef struct _odpdrv_device_s _odpdrv_device_t;
> +typedef struct _odpdrv_devio_s _odpdrv_devio_t;
>
>  /* an enumerator class (list element) */
>  struct _odpdrv_enumr_class_s {
> @@ -72,6 +73,20 @@ typedef struct _odpdrv_device_lst_t {
>  } _odpdrv_device_lst_t;
>  static struct _odpdrv_device_lst_t device_lst;
>
> +/* a devio (list element) */
> +struct _odpdrv_devio_s {
> +   odpdrv_devio_param_t param;
> +   _odp_ishm_pool_t *pool;
> +   struct _odpdrv_devio_s *next;
> +} _odpdrv_devio_s;
> +
> +/* the devio list: */
> +typedef struct _odpdrv_devio_lst_t {
> +   odp_rwlock_recursive_t lock;
> +   _odpdrv_devio_t *head;
> +} _odpdrv_devio_lst_t;
> +static struct _odpdrv_devio_lst_t devio_lst;
> +
>  /* some driver elements (such as enumeraor classes, drivers, devio) may
>   * register before init_global and init_local complete. Mutex will fail
>   * in this cases but should be used later on.
> @@ -149,6 +164,30 @@ static void dev_list_write_unlock(void)
> odp_rwlock_recursive_write_unlock(_lst.lock);
>  }
>
> +static void devio_list_read_lock(void)
> +{
> +   if (init_global_status == DONE)
>

Same comment on the need for these guards.


> +   odp_rwlock_recursive_read_lock(_lst.lock);
> +}
> +
> +static void devio_list_read_unlock(void)
> +{
> +   if (init_global_status == DONE)
> +   odp_rwlock_recursive_read_unlock(_lst.lock);
> +}
> +
> +static void devio_list_write_lock(void)
> +{
> +   if (init_global_status == DONE)
> +   odp_rwlock_recursive_write_lock(_lst.lock);
> +}
> +
> +static void devio_list_write_unlock(void)
> +{
> +   if (init_global_status == DONE)
> +   odp_rwlock_recursive_write_unlock(_lst.lock);
> +}
> +
>  odpdrv_enumr_class_t odpdrv_enumr_class_register(od
> pdrv_enumr_class_param_t
>  *param)
>  {
> @@ -415,10 +454,65 @@ odpdrv_device_t *odpdrv_device_query(odpdrv_enumr_t
> enumr, const char *address)
>
>  odpdrv_devio_t odpdrv_devio_register(odpdrv_devio_param_t *param)
>  {
> -   ODP_ERR("NOT Supported yet! Driver %s Registration!\n.",
> -   param->api_name);
> +   _odpdrv_devio_t *devio;
> +
> +   /* parse the list of already registered devios to make
> +* sure no devio providing the same interface using th esame
> enumerator
> +* already exists:
> +*/
> +   devio_list_read_lock();
> +   devio = devio_lst.head;
>

Same comment on needing to initialize the head field (should be done at
init time along with the lock).


> +   while (devio) {
> +   if ((strncmp(param->api_name, devio->param.api_name,
> +ODPDRV_NAME_SIZE) == 0) &&
> +   (strncmp(param->enumr_api_name,
> devio->param.enumr_api_name,
> +ODPDRV_NAME_SIZE) == 0)) {
> +   ODP_ERR("a devio providing interface '%s' for
> devices "
> +   "of type '%s' is already registered\n!",
> +   param->api_name, param->enumr_api_name);
> +   devio_list_read_unlock();
> +   return ODPDRV_DEVIO_INVALID;
> +   }
> +   devio = devio->next;
> +   }
> +   devio_list_read_unlock();
>
> -   return ODPDRV_DEVIO_INVALID;
> +   /* allocate memory for the new devio:
> +* If init_global has not been done yet, then, we cannot allocate
> +* from any _ishm pool (ishm has not even been initialised at this
> +* stage...this happens when statically linked devios
> +* register: their __constructor__ function is run before main()

+* is called). But any malloc performed here(before init_global)
> +* 

[lng-odp] [API-NEXT PATCH 13/21] linux-gen: drv: devio registration

[Christophe Milard]

devios (dev IO) provide a interface for drivers to access a device:
Devices enumerated by enumerators may be accessed in by different
mechanisms (depending on iommu presence or other factors). This extra
abstraction is provided by devios, which provide a sets of methods to
access the devices of a given type (i.e. registred enumerator(s)
enumerating devices of the same kind (e.g. PCI)).
This patch just implements the devio registration method provided by the
driver API.

Signed-off-by: Christophe Milard 
---
 platform/linux-generic/drv_driver.c | 134 +++-
 1 file changed, 131 insertions(+), 3 deletions(-)

diff --git a/platform/linux-generic/drv_driver.c 
b/platform/linux-generic/drv_driver.c
index 517a3c6..eb0dc48 100644
--- a/platform/linux-generic/drv_driver.c
+++ b/platform/linux-generic/drv_driver.c
@@ -28,6 +28,7 @@ static _odp_ishm_pool_t *list_elt_pool;
 typedef struct _odpdrv_enumr_class_s _odpdrv_enumr_class_t;
 typedef struct _odpdrv_enumr_s _odpdrv_enumr_t;
 typedef struct _odpdrv_device_s _odpdrv_device_t;
+typedef struct _odpdrv_devio_s _odpdrv_devio_t;
 
 /* an enumerator class (list element) */
 struct _odpdrv_enumr_class_s {
@@ -72,6 +73,20 @@ typedef struct _odpdrv_device_lst_t {
 } _odpdrv_device_lst_t;
 static struct _odpdrv_device_lst_t device_lst;
 
+/* a devio (list element) */
+struct _odpdrv_devio_s {
+   odpdrv_devio_param_t param;
+   _odp_ishm_pool_t *pool;
+   struct _odpdrv_devio_s *next;
+} _odpdrv_devio_s;
+
+/* the devio list: */
+typedef struct _odpdrv_devio_lst_t {
+   odp_rwlock_recursive_t lock;
+   _odpdrv_devio_t *head;
+} _odpdrv_devio_lst_t;
+static struct _odpdrv_devio_lst_t devio_lst;
+
 /* some driver elements (such as enumeraor classes, drivers, devio) may
  * register before init_global and init_local complete. Mutex will fail
  * in this cases but should be used later on.
@@ -149,6 +164,30 @@ static void dev_list_write_unlock(void)
odp_rwlock_recursive_write_unlock(_lst.lock);
 }
 
+static void devio_list_read_lock(void)
+{
+   if (init_global_status == DONE)
+   odp_rwlock_recursive_read_lock(_lst.lock);
+}
+
+static void devio_list_read_unlock(void)
+{
+   if (init_global_status == DONE)
+   odp_rwlock_recursive_read_unlock(_lst.lock);
+}
+
+static void devio_list_write_lock(void)
+{
+   if (init_global_status == DONE)
+   odp_rwlock_recursive_write_lock(_lst.lock);
+}
+
+static void devio_list_write_unlock(void)
+{
+   if (init_global_status == DONE)
+   odp_rwlock_recursive_write_unlock(_lst.lock);
+}
+
 odpdrv_enumr_class_t odpdrv_enumr_class_register(odpdrv_enumr_class_param_t
 *param)
 {
@@ -415,10 +454,65 @@ odpdrv_device_t *odpdrv_device_query(odpdrv_enumr_t 
enumr, const char *address)
 
 odpdrv_devio_t odpdrv_devio_register(odpdrv_devio_param_t *param)
 {
-   ODP_ERR("NOT Supported yet! Driver %s Registration!\n.",
-   param->api_name);
+   _odpdrv_devio_t *devio;
+
+   /* parse the list of already registered devios to make
+* sure no devio providing the same interface using th esame enumerator
+* already exists:
+*/
+   devio_list_read_lock();
+   devio = devio_lst.head;
+   while (devio) {
+   if ((strncmp(param->api_name, devio->param.api_name,
+ODPDRV_NAME_SIZE) == 0) &&
+   (strncmp(param->enumr_api_name, devio->param.enumr_api_name,
+ODPDRV_NAME_SIZE) == 0)) {
+   ODP_ERR("a devio providing interface '%s' for devices "
+   "of type '%s' is already registered\n!",
+   param->api_name, param->enumr_api_name);
+   devio_list_read_unlock();
+   return ODPDRV_DEVIO_INVALID;
+   }
+   devio = devio->next;
+   }
+   devio_list_read_unlock();
 
-   return ODPDRV_DEVIO_INVALID;
+   /* allocate memory for the new devio:
+* If init_global has not been done yet, then, we cannot allocate
+* from any _ishm pool (ishm has not even been initialised at this
+* stage...this happens when statically linked devios
+* register: their __constructor__ function is run before main()
+* is called). But any malloc performed here(before init_global)
+* will be inherited by any odpthreads (process or pthreads) as we
+* are still running in the ODP instantiation processes and all
+* other processes are guaranteed to be descendent of this one...
+* If init_global has been done, then we allocate from the _ishm pool
+* to guarantee visibility from any ODP thread.
+*/
+
+   if (init_global_status == UNDONE) {
+   devio = malloc(sizeof(_odpdrv_devio_t));
+   if