Re: [lng-odp] [API-NEXT PATCH 1/3] drv: shm: function to query for physical addresses

2016-11-11 Thread Christophe Milard
On 10 November 2016 at 20:05, Maxim Uvarov  wrote:
> On 11/10/16 20:21, Christophe Milard wrote:
>>
>> The capability "can_getphy" is introduced and tells whether physical
>> address queries are available.
>> The function odpdrv_getphy() is added to query for physical address
>> (from virtual address)
>>
>> Signed-off-by: Christophe Milard 
>> ---
>>   include/odp/drv/spec/shm.h | 35 +++
>>   1 file changed, 35 insertions(+)
>>
>> diff --git a/include/odp/drv/spec/shm.h b/include/odp/drv/spec/shm.h
>> index ef64f5d..ca3aae2 100644
>> --- a/include/odp/drv/spec/shm.h
>> +++ b/include/odp/drv/spec/shm.h
>> @@ -43,6 +43,12 @@ extern "C" {
>>   #define ODPDRV_SHM_LOCK   0x02 /**< Memory shall be locked
>> (no swap) */
>> /**
>> + * @typedef odpdrv_phys_addr_t
>> + * A physical memory address
>> + *
>> + */
>> +
>> +/**
>>* Shared memory block info
>>*/
>>   typedef struct odpdrv_shm_info_t {
>> @@ -75,6 +81,13 @@ typedef struct odpdrv_shm_capability_t {
>>  * available memory size. */
>> uint64_t max_align;
>>   + /** Can retrieve physical addresses
>> +*
>> +* A true (non-zero) value means that odpdrv_getphy() can be
>> +* used to retrieve physical memory addresses, (as well as
>> +* the debug function odp_drv_dumpphy() */
>> +   int can_getphy;
>
> intnphys_addr:1;

yes, of course =>V2

>>
>> +
>>   } odpdrv_shm_capability_t;
>> /**
>> @@ -220,6 +233,28 @@ int odpdrv_shm_print_all(const char *title);
>>   uint64_t odpdrv_shm_to_u64(odpdrv_shm_t hdl);
>> /**
>> + * Get physical address from virtual address
>> + *
>> + * @addr   virtual address.
>> + * @return Physicall address or NULL
>> + *
>> + * @note This routine will work only if capability "can_getphy" is true.
>> + */
>> +odpdrv_phys_addr_t odpdrv_getphy(const void *addr);
>
>
> naming is bad. By 'phy' I used to see physics layer chip, not physical
> memory.
> Something similar to linux kernel might be:
>
> odpdrv_virt_to_phys()

OK => V2
> odpdrv_phys_to_virt()

does not exist yet. no real need as far as I can see.

>
>
> and (I don't know how writes done from user space), but probably you need
> odp version of ioremap()
>
>
>> +
>> +/**
>> + * Print physical address mapping
>> + *
>
> print where?

It is said is the descritpion body:uses ODP_DBG
I will add --enable-debug-print required too

>>
>> + * @addr   virtual address.
>
> what is len?

copy/paste error thanks for the pick =>V2

>
>> + * @return Physicall address or NULL
>> + *
>
>
> it's void, no return.

copy/paste error thanks for the pick =>V2

>
>> + * @note This routine will work only if capability "can_getphy" is true.
>> + * @note This routine is intended to be used for diagnostic purposes,
>> + * and uses ODP_DBG printouts
>> + */
>> +void odpdrv_dumpphy(void *addr, uint64_t len);
>
>
> Maybe odpdrv_memmap_print()? to print current mappings by driver?

OK =>V2

>
> should driver handle be passed as argument?

I don't think so: first, it is less flexible: if a driver for any
reason would need to allocate a large memory area and worry about its
physycal fragmentation, then it would not be happy with just handles.
If a driver starts to split a page in different fragments, then that
single handle would not be very nice to deal with.
Drivers will care much more about the address than the handle (I guess
even that some drivers may just ignore the handle completely as memory
can be released by address (when single VA flag is set))
The excelent name you suggested, virt_to_phy, would loose some of ots
prestige! ;-)

Thanks for your comments!

Christophe.
>
>
>> +
>> +/**
>>* @}
>>*/
>>
>
>


Re: [lng-odp] [API-NEXT PATCH 1/3] drv: shm: function to query for physical addresses

2016-11-10 Thread Francois Ozog
On 10 November 2016 at 20:05, Maxim Uvarov  wrote:

> On 11/10/16 20:21, Christophe Milard wrote:
>
>> The capability "can_getphy" is introduced and tells whether physical
>> address queries are available.
>> The function odpdrv_getphy() is added to query for physical address
>> (from virtual address)
>>
>> Signed-off-by: Christophe Milard 
>> ---
>>   include/odp/drv/spec/shm.h | 35 +++
>>   1 file changed, 35 insertions(+)
>>
>> diff --git a/include/odp/drv/spec/shm.h b/include/odp/drv/spec/shm.h
>> index ef64f5d..ca3aae2 100644
>> --- a/include/odp/drv/spec/shm.h
>> +++ b/include/odp/drv/spec/shm.h
>> @@ -43,6 +43,12 @@ extern "C" {
>>   #define ODPDRV_SHM_LOCK   0x02 /**< Memory shall be locked
>> (no swap) */
>> /**
>> + * @typedef odpdrv_phys_addr_t
>> + * A physical memory address
>> + *
>> + */
>> +
>> +/**
>>* Shared memory block info
>>*/
>>   typedef struct odpdrv_shm_info_t {
>> @@ -75,6 +81,13 @@ typedef struct odpdrv_shm_capability_t {
>>  * available memory size. */
>> uint64_t max_align;
>>   + /** Can retrieve physical addresses
>> +*
>> +* A true (non-zero) value means that odpdrv_getphy() can be
>> +* used to retrieve physical memory addresses, (as well as
>> +* the debug function odp_drv_dumpphy() */
>> +   int can_getphy;
>>
> intnphys_addr:1;
>
>> +
>>   } odpdrv_shm_capability_t;
>> /**
>> @@ -220,6 +233,28 @@ int odpdrv_shm_print_all(const char *title);
>>   uint64_t odpdrv_shm_to_u64(odpdrv_shm_t hdl);
>> /**
>> + * Get physical address from virtual address
>> + *
>> + * @addr   virtual address.
>> + * @return Physicall address or NULL
>> + *
>> + * @note This routine will work only if capability "can_getphy" is true.
>> + */
>> +odpdrv_phys_addr_t odpdrv_getphy(const void *addr);
>>
>
> naming is bad. By 'phy' I used to see physics layer chip, not physical
> memory.
> Something similar to linux kernel might be:
>
> odpdrv_virt_to_phys()
> odpdrv_phys_to_virt()
>
>
> and (I don't know how writes done from user space), but probably you need
> odp version of ioremap()
>
> [FF]: I agree for the naming. "physical memory" is then handled through
either UIO or VFIO frameworks which deal with IO particularities, we should
only use APIs from them (ioremap is a kernel API). Drivers may have to
implement one or the other or both physical access methods. Those
frameworks provide required functions to prepare DMAs properly (on some
architectures "DMA from device" and "DMA to device" require preparation
even in bare metal). VFIO is the preferred method because it is secure but
it is not yet widely available. For instance, DPDK virtio-net guest and
host drivers are using uio. vfio support is embryonic and provides poor
performance at the moment.

>
> +
>> +/**
>> + * Print physical address mapping
>> + *
>>
> print where?
>
>> + * @addr   virtual address.
>>
> what is len?
>
> + * @return Physicall address or NULL
>> + *
>>
>
> it's void, no return.
>
> + * @note This routine will work only if capability "can_getphy" is true.
>> + * @note This routine is intended to be used for diagnostic purposes,
>> + * and uses ODP_DBG printouts
>> + */
>> +void odpdrv_dumpphy(void *addr, uint64_t len);
>>
>
> Maybe odpdrv_memmap_print()? to print current mappings by driver?
>
> should driver handle be passed as argument?
>
>
> +
>> +/**
>>* @}
>>*/
>>
>>
>
>


-- 
[image: Linaro] 
François-Frédéric Ozog | *Director Linaro Networking Group*
T: +33.67221.6485
francois.o...@linaro.org | Skype: ffozog


Re: [lng-odp] [API-NEXT PATCH 1/3] drv: shm: function to query for physical addresses

2016-11-10 Thread Maxim Uvarov

On 11/10/16 20:21, Christophe Milard wrote:

The capability "can_getphy" is introduced and tells whether physical
address queries are available.
The function odpdrv_getphy() is added to query for physical address
(from virtual address)

Signed-off-by: Christophe Milard 
---
  include/odp/drv/spec/shm.h | 35 +++
  1 file changed, 35 insertions(+)

diff --git a/include/odp/drv/spec/shm.h b/include/odp/drv/spec/shm.h
index ef64f5d..ca3aae2 100644
--- a/include/odp/drv/spec/shm.h
+++ b/include/odp/drv/spec/shm.h
@@ -43,6 +43,12 @@ extern "C" {
  #define ODPDRV_SHM_LOCK   0x02 /**< Memory shall be locked (no 
swap) */
  
  /**

+ * @typedef odpdrv_phys_addr_t
+ * A physical memory address
+ *
+ */
+
+/**
   * Shared memory block info
   */
  typedef struct odpdrv_shm_info_t {
@@ -75,6 +81,13 @@ typedef struct odpdrv_shm_capability_t {
 * available memory size. */
uint64_t max_align;
  
+	/** Can retrieve physical addresses

+*
+* A true (non-zero) value means that odpdrv_getphy() can be
+* used to retrieve physical memory addresses, (as well as
+* the debug function odp_drv_dumpphy() */
+   int can_getphy;

intnphys_addr:1;

+
  } odpdrv_shm_capability_t;
  
  /**

@@ -220,6 +233,28 @@ int odpdrv_shm_print_all(const char *title);
  uint64_t odpdrv_shm_to_u64(odpdrv_shm_t hdl);
  
  /**

+ * Get physical address from virtual address
+ *
+ * @addr   virtual address.
+ * @return Physicall address or NULL
+ *
+ * @note This routine will work only if capability "can_getphy" is true.
+ */
+odpdrv_phys_addr_t odpdrv_getphy(const void *addr);


naming is bad. By 'phy' I used to see physics layer chip, not physical 
memory.

Something similar to linux kernel might be:

odpdrv_virt_to_phys()
odpdrv_phys_to_virt()


and (I don't know how writes done from user space), but probably you 
need odp version of ioremap()




+
+/**
+ * Print physical address mapping
+ *

print where?

+ * @addr   virtual address.

what is len?


+ * @return Physicall address or NULL
+ *


it's void, no return.


+ * @note This routine will work only if capability "can_getphy" is true.
+ * @note This routine is intended to be used for diagnostic purposes,
+ * and uses ODP_DBG printouts
+ */
+void odpdrv_dumpphy(void *addr, uint64_t len);


Maybe odpdrv_memmap_print()? to print current mappings by driver?

should driver handle be passed as argument?


+
+/**
   * @}
   */
  




[lng-odp] [API-NEXT PATCH 1/3] drv: shm: function to query for physical addresses

2016-11-10 Thread Christophe Milard
The capability "can_getphy" is introduced and tells whether physical
address queries are available.
The function odpdrv_getphy() is added to query for physical address
(from virtual address)

Signed-off-by: Christophe Milard 
---
 include/odp/drv/spec/shm.h | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/include/odp/drv/spec/shm.h b/include/odp/drv/spec/shm.h
index ef64f5d..ca3aae2 100644
--- a/include/odp/drv/spec/shm.h
+++ b/include/odp/drv/spec/shm.h
@@ -43,6 +43,12 @@ extern "C" {
 #define ODPDRV_SHM_LOCK0x02 /**< Memory shall be locked (no 
swap) */
 
 /**
+ * @typedef odpdrv_phys_addr_t
+ * A physical memory address
+ *
+ */
+
+/**
  * Shared memory block info
  */
 typedef struct odpdrv_shm_info_t {
@@ -75,6 +81,13 @@ typedef struct odpdrv_shm_capability_t {
 * available memory size. */
uint64_t max_align;
 
+   /** Can retrieve physical addresses
+*
+* A true (non-zero) value means that odpdrv_getphy() can be
+* used to retrieve physical memory addresses, (as well as
+* the debug function odp_drv_dumpphy() */
+   int can_getphy;
+
 } odpdrv_shm_capability_t;
 
 /**
@@ -220,6 +233,28 @@ int odpdrv_shm_print_all(const char *title);
 uint64_t odpdrv_shm_to_u64(odpdrv_shm_t hdl);
 
 /**
+ * Get physical address from virtual address
+ *
+ * @addr   virtual address.
+ * @return Physicall address or NULL
+ *
+ * @note This routine will work only if capability "can_getphy" is true.
+ */
+odpdrv_phys_addr_t odpdrv_getphy(const void *addr);
+
+/**
+ * Print physical address mapping
+ *
+ * @addr   virtual address.
+ * @return Physicall address or NULL
+ *
+ * @note This routine will work only if capability "can_getphy" is true.
+ * @note This routine is intended to be used for diagnostic purposes,
+ * and uses ODP_DBG printouts
+ */
+void odpdrv_dumpphy(void *addr, uint64_t len);
+
+/**
  * @}
  */
 
-- 
2.7.4