Re: [dpdk-dev] [PATCH v2 3/5] eal/memory: rename memseg member phys to iova addr

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 05-Sep-17 11:31 AM, Santosh Shukla wrote:
> Renaming rte_memseg {.phys_addr} to {.iova_addr}
> 
> Signed-off-by: Santosh Shukla <santosh.shukla at caviumnetworks.com>
> ---
> v1 --> v2:
> - includes freebsdp v1 build fixes.
> 
>   lib/librte_eal/bsdapp/eal/eal_memory.c     | 4 ++--
>   lib/librte_eal/common/eal_common_memory.c  | 2 +-
>   lib/librte_eal/common/include/rte_memory.h | 4 ++--
>   lib/librte_eal/common/rte_malloc.c         | 5 +++--
>   lib/librte_eal/linuxapp/eal/eal_memory.c   | 8 ++++----
>   lib/librte_eal/linuxapp/eal/eal_vfio.c     | 4 ++--
>   6 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c b/lib/librte_eal/bsdapp/eal/eal_memory.c
> index 10c2e121f..d8882dcef 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_memory.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_memory.c
> @@ -73,7 +73,7 @@ rte_eal_hugepage_init(void)
>   	/* for debug purposes, hugetlbfs can be disabled */
>   	if (internal_config.no_hugetlbfs) {
>   		addr = malloc(internal_config.memory);
> -		mcfg->memseg[0].phys_addr = (iova_addr_t)(uintptr_t)addr;
> +		mcfg->memseg[0].iova_addr = (iova_addr_t)(uintptr_t)addr;
>   		mcfg->memseg[0].addr = addr;
>   		mcfg->memseg[0].hugepage_sz = RTE_PGSIZE_4K;
>   		mcfg->memseg[0].len = internal_config.memory;
> @@ -114,7 +114,7 @@ rte_eal_hugepage_init(void)
>   
>   			seg = &mcfg->memseg[seg_idx++];
>   			seg->addr = addr;
> -			seg->phys_addr = physaddr;
> +			seg->iova_addr = (iova_addr_t)physaddr;
>   			seg->hugepage_sz = hpi->hugepage_sz;
>   			seg->len = hpi->hugepage_sz;
>   			seg->nchannel = mcfg->nchannel;
> diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
> index 996877ef5..5ed83d20a 100644
> --- a/lib/librte_eal/common/eal_common_memory.c
> +++ b/lib/librte_eal/common/eal_common_memory.c
> @@ -100,7 +100,7 @@ rte_dump_physmem_layout(FILE *f)
>   		       "virt:%p, socket_id:%"PRId32", "
>   		       "hugepage_sz:%"PRIu64", nchannel:%"PRIx32", "
>   		       "nrank:%"PRIx32"\n", i,
> -		       mcfg->memseg[i].phys_addr,
> +		       mcfg->memseg[i].iova_addr,
>   		       mcfg->memseg[i].len,
>   		       mcfg->memseg[i].addr,
>   		       mcfg->memseg[i].socket_id,
> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
> index 5face8c86..6b148ba8e 100644
> --- a/lib/librte_eal/common/include/rte_memory.h
> +++ b/lib/librte_eal/common/include/rte_memory.h
> @@ -98,14 +98,14 @@ enum rte_page_sizes {
>    */
>   #define __rte_cache_min_aligned __rte_aligned(RTE_CACHE_LINE_MIN_SIZE)
>   
> -typedef uint64_t iova_addr_t; /**< Physical address definition. */
> +typedef uint64_t iova_addr_t; /**< Iova address definition. */
>   #define RTE_BAD_PHYS_ADDR ((iova_addr_t)-1)
>   
>   /**
>    * Physical memory segment descriptor.
>    */
>   struct rte_memseg {
> -	iova_addr_t phys_addr;      /**< Start physical address. */
> +	iova_addr_t iova_addr;      /**< Start iova(_pa or _va) address. */
>   	RTE_STD_C11
>   	union {
>   		void *addr;         /**< Start virtual address. */
> diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
> index 3ce6034bf..b65a06f9d 100644
> --- a/lib/librte_eal/common/rte_malloc.c
> +++ b/lib/librte_eal/common/rte_malloc.c
> @@ -254,7 +254,8 @@ rte_malloc_virt2phy(const void *addr)
>   	const struct malloc_elem *elem = malloc_elem_from_data(addr);
>   	if (elem == NULL)
>   		return RTE_BAD_PHYS_ADDR;
> -	if (elem->ms->phys_addr == RTE_BAD_PHYS_ADDR)
> +	if (elem->ms->iova_addr == RTE_BAD_PHYS_ADDR)
>   		return RTE_BAD_PHYS_ADDR;
> -	return elem->ms->phys_addr + ((uintptr_t)addr - (uintptr_t)elem->ms->addr);
> +	return elem->ms->iova_addr +
> +		((uintptr_t)addr - (uintptr_t)elem->ms->addr);

Hi Santosh,

I understand that this is a simple search-replace patch, but maybe you 
could go ahead and fix this to be RTE_PTR_DIFF, like in below case.

>   }
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 405c15bcd..5d9702c72 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -154,7 +154,7 @@ rte_mem_virt2phy(const void *virtaddr)
>   			if (virtaddr > memseg->addr &&
>   					virtaddr < RTE_PTR_ADD(memseg->addr,
>   						memseg->len)) {
> -				return memseg->phys_addr +
> +				return memseg->iova_addr +
>   					RTE_PTR_DIFF(virtaddr, memseg->addr);
>   			}
>   		}
> @@ -1059,7 +1059,7 @@ rte_eal_hugepage_init(void)
>   					strerror(errno));
>   			return -1;
>   		}
> -		mcfg->memseg[0].phys_addr = RTE_BAD_PHYS_ADDR;
> +		mcfg->memseg[0].iova_addr = RTE_BAD_PHYS_ADDR;
>   		mcfg->memseg[0].addr = addr;
>   		mcfg->memseg[0].hugepage_sz = RTE_PGSIZE_4K;
>   		mcfg->memseg[0].len = internal_config.memory;
> @@ -1319,7 +1319,7 @@ rte_eal_hugepage_init(void)
>   			if (j == RTE_MAX_MEMSEG)
>   				break;
>   
> -			mcfg->memseg[j].phys_addr = hugepage[i].physaddr;
> +			mcfg->memseg[j].iova_addr = hugepage[i].physaddr;
>   			mcfg->memseg[j].addr = hugepage[i].final_va;
>   			mcfg->memseg[j].len = hugepage[i].size;
>   			mcfg->memseg[j].socket_id = hugepage[i].socket_id;
> @@ -1330,7 +1330,7 @@ rte_eal_hugepage_init(void)
>   #ifdef RTE_ARCH_PPC_64
>   		/* Use the phy and virt address of the last page as segment
>   		 * address for IBM Power architecture */
> -			mcfg->memseg[j].phys_addr = hugepage[i].physaddr;
> +			mcfg->memseg[j].iova_addr = hugepage[i].physaddr;
>   			mcfg->memseg[j].addr = hugepage[i].final_va;
>   #endif
>   			mcfg->memseg[j].len += mcfg->memseg[j].hugepage_sz;
> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> index 946df7e31..c03fd713c 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> @@ -706,7 +706,7 @@ vfio_type1_dma_map(int vfio_container_fd)
>   		dma_map.argsz = sizeof(struct vfio_iommu_type1_dma_map);
>   		dma_map.vaddr = ms[i].addr_64;
>   		dma_map.size = ms[i].len;
> -		dma_map.iova = ms[i].phys_addr;
> +		dma_map.iova = ms[i].iova_addr;
>   		dma_map.flags = VFIO_DMA_MAP_FLAG_READ | VFIO_DMA_MAP_FLAG_WRITE;
>   
>   		ret = ioctl(vfio_container_fd, VFIO_IOMMU_MAP_DMA, &dma_map);
> @@ -792,7 +792,7 @@ vfio_spapr_dma_map(int vfio_container_fd)
>   		dma_map.argsz = sizeof(struct vfio_iommu_type1_dma_map);
>   		dma_map.vaddr = ms[i].addr_64;
>   		dma_map.size = ms[i].len;
> -		dma_map.iova = ms[i].phys_addr;
> +		dma_map.iova = ms[i].iova_addr;
>   		dma_map.flags = VFIO_DMA_MAP_FLAG_READ |
>   				 VFIO_DMA_MAP_FLAG_WRITE;
>   
> 


-- 
Thanks,
Anatoly