On 10 November 2016 at 20:05, Maxim Uvarov <maxim.uva...@linaro.org> 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 <christophe.mil...@linaro.org>
>> ---
>> 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.
>
>
>> +
>> +/**
>> * @}
>> */
>>
>
>
