Re: [dpdk-dev] [PATCH v2 2/5] EAL: Add new EAL "--qtest-virtio" option

[Tetsuya Mukawa <mukawa@igel.co.jp>]

On 2016/02/16 20:36, Tan, Jianfeng wrote:
> Hi David,
>
> On 2/16/2016 1:53 PM, David Marchand wrote:
>> On Wed, Feb 10, 2016 at 4:40 AM, Tetsuya Mukawa <mukawa@igel.co.jp>
>> wrote:
>>> To work with qtest virtio-net PMD, virtual address that maps hugepages
>>> should be between (1 << 31) to (1 << 44). This patch adds one more
>>> option
>>> to map like this. Also all hugepages should consists of one file.
>>> Because of this, the option will work only when '--single-file'
>>> option is
>>> specified.
>> This patch is pure virtio stuff.
>> Please, rework this so that we have a generic api in eal (asking for a
>> free region could be of use for something else).
>> Then you can call this api from virtio pmd.
>>
>> If you need to pass options to virtio pmd, add some devargs for it.
>>
>
> Seems it's hard to slip this option into --vdev="eth_qtest_virtio0..."
> from my side because memory initialization happens before vdev option
> is parsed.
>
> Can we make use of "--base-virtaddr" achieve the function of this option?

I think same thing also.

Option1 is just using "--base-virtaddr" option without any fixes.

Option2 is adding "--range-virtaddr" option.
When "--range-option" is set, EAL will find free region in specified
memory region, then set the address to 'base_vrtiaddr' variable.
This will be done before base_addr variable is used by current
implementation.
How about this?

Here is rough implementation.

diff --git a/lib/librte_eal/common/eal_common_options.c
b/lib/librte_eal/common/eal_common_options.c
index 65bccbd..996b61d 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -74,6 +74,8 @@ eal_short_options[] =
 const struct option
 eal_long_options[] = {
        {OPT_BASE_VIRTADDR,     1, NULL, OPT_BASE_VIRTADDR_NUM    },
+       {OPT_RANGE_VIRTADDR,    1, NULL, OPT_RANGE_VIRTADDR_NUM   },
+       {OPT_ALIGN_MEMSIZE,     0, NULL, OPT_ALIGN_MEMSIZE_NUM    },
        {OPT_CREATE_UIO_DEV,    0, NULL, OPT_CREATE_UIO_DEV_NUM   },
        {OPT_FILE_PREFIX,       1, NULL, OPT_FILE_PREFIX_NUM      },
        {OPT_HELP,              0, NULL, OPT_HELP_NUM             },
@@ -137,6 +139,9 @@ eal_reset_internal_config(struct internal_config
*internal_cfg)
        for (i = 0; i < MAX_HUGEPAGE_SIZES; i++)
                internal_cfg->hugepage_info[i].lock_descriptor = -1;
        internal_cfg->base_virtaddr = 0;
+       internal_cfg->range_virtaddr_start = 0;
+       internal_cfg->range_virtaddr_end = 0;
+       internal_cfg->align_memsize = 0;

        internal_cfg->syslog_facility = LOG_DAEMON;
        /* default value from build option */
@@ -985,6 +990,18 @@ eal_check_common_options(struct internal_config
*internal_cfg)
                return -1;
        }

+       if (internal_cfg->base_virtaddr &&
internal_cfg->range_virtaddr_end) {
+               RTE_LOG(ERR, EAL, "Option --"OPT_RANGE_VIRTADDR" cannot "
+                       "be specified together with
--"OPT_BASE_VIRTADDR"\n");
+               return -1;
+       }
+
+       if (internal_cfg->range_virtaddr_end !=0 &&
internal_cfg->align_memsize) {
+               RTE_LOG(ERR, EAL, "Option --"OPT_RANGE_VIRTADDR" should be "
+                       "specified together with --"OPT_ALIGN_MEMSIZE"\n");
+               return -1;
+       }
+
        return 0;
 }

diff --git a/lib/librte_eal/common/eal_internal_cfg.h
b/lib/librte_eal/common/eal_internal_cfg.h
index 9117ed9..df33a9f 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -78,6 +78,9 @@ struct internal_config {
        volatile unsigned force_sockets;
        volatile uint64_t socket_mem[RTE_MAX_NUMA_NODES]; /**< amount of
memory per socket */
        uintptr_t base_virtaddr;          /**< base address to try and
reserve memory from */
+       uintptr_t range_virtaddr_start;   /**< start address of mappable
region */
+       uintptr_t range_virtaddr_end;     /**< end address of mappable
region */
+       volatile unsigned align_memsize;  /**< true to align virtaddr by
memory size */
        volatile int syslog_facility;     /**< facility passed to
openlog() */
        volatile uint32_t log_level;      /**< default log level */
        /** default interrupt mode for VFIO */
diff --git a/lib/librte_eal/common/eal_options.h
b/lib/librte_eal/common/eal_options.h
index e5da14a..9e36f68 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -47,6 +47,10 @@ enum {
        OPT_LONG_MIN_NUM = 256,
 #define OPT_BASE_VIRTADDR     "base-virtaddr"
        OPT_BASE_VIRTADDR_NUM,
+#define OPT_RANGE_VIRTADDR    "range-virtaddr"
+       OPT_RANGE_VIRTADDR_NUM,
+#define OPT_ALIGN_MEMSIZE     "align-memsize"
+       OPT_ALIGN_MEMSIZE_NUM,
 #define OPT_CREATE_UIO_DEV    "create-uio-dev"
        OPT_CREATE_UIO_DEV_NUM,
 #define OPT_FILE_PREFIX       "file-prefix"
diff --git a/lib/librte_eal/linuxapp/eal/eal.c
b/lib/librte_eal/linuxapp/eal/eal.c
index 82f34f7..e6c6b34 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -444,6 +444,38 @@ eal_parse_base_virtaddr(const char *arg)
 }

 static int
+eal_parse_range_virtaddr(const char *range)
+{
+       char *p, *endptr;
+       uint64_t tmp_start, tmp_end;
+
+       if (range[0] == '\0')
+               return -1;
+
+       p = strchr(range, '-');
+       if (p == NULL)
+               return -1;
+       *p++ = '\0';
+
+       errno = 0;
+       tmp_start = strtoul(range, &endptr, 0);
+       if ((errno != 0) || endptr == NULL || (*endptr != '\0'))
+               return -1;
+
+       tmp_end = strtoul(p, &endptr, 0);
+       if ((errno != 0) || endptr == NULL || (*endptr != '\0'))
+               return -1;
+
+       if (tmp_start >= tmp_end)
+               return -1;
+
+       internal_config.range_virtaddr_start = tmp_start;
+       internal_config.range_virtaddr_end = tmp_end;
+
+       return 0;
+}
+
+static int
 eal_parse_vfio_intr(const char *mode)
 {
        unsigned i;
@@ -604,6 +636,20 @@ eal_parse_args(int argc, char **argv)
                        }
                        break;

+               case OPT_RANGE_VIRTADDR_NUM:
+                       if (eal_parse_range_virtaddr(optarg) < 0) {
+                               RTE_LOG(ERR, EAL, "invalid parameter for --"
+                                               OPT_RANGE_VIRTADDR "\n");
+                               eal_usage(prgname);
+                               ret = -1;
+                               goto out;
+                       }
+                       break;
+
+               case OPT_ALIGN_MEMSIZE_NUM:
+                       internal_config.align_memsize = 1;
+                       break;
+
                case OPT_VFIO_INTR_NUM:
                        if (eal_parse_vfio_intr(optarg) < 0) {
                                RTE_LOG(ERR, EAL, "invalid parameters
for --"
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c
b/lib/librte_eal/linuxapp/eal/eal_memory.c
index a6b3616..a9d30d7 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -251,6 +251,75 @@ aslr_enabled(void)
 }

 /*
+ * Find memory space that fits user request.
+ */
+static uintptr_t
+rte_eal_get_free_region(uint64_t pagesz)
+{
+       uint64_t alloc_size, start, end, next_start;
+       uint64_t low_limit, high_limit;
+       uintptr_t addr = 0;
+       char buf[1024], *p;
+       FILE *fp;
+
+       alloc_size = internal_config.memory;
+       low_limit = internal_config.range_virtaddr_start;
+       high_limit = internal_config.range_virtaddr_end;
+
+       /* allocation size should be aligned by page size */
+       if (alloc_size != RTE_ALIGN_CEIL(alloc_size, pagesz)) {
+               rte_panic("Invalid allocation size 0x%lx\n", alloc_size);
+               return NULL;
+       }
+
+       if (internal_config.align_memsize) {
+               /*
+                * address should be aligned by allocation size because
+                * BAR register of PCI device requiers such an address
+                */
+               low_limit = RTE_ALIGN_CEIL(low_limit, alloc_size);
+               high_limit = RTE_ALIGN_FLOOR(high_limit, alloc_size);
+       }
+
+       fp = fopen("/proc/self/maps", "r");
+       if (fp == NULL) {
+               rte_panic("Cannot open /proc/self/maps\n");
+               return NULL;
+       }
+
+       next_start = 0;
+       do {
+               start = next_start;
+
+               if ((p = fgets(buf, sizeof(buf), fp)) != NULL) {
+                       if (sscanf(p, "%lx-%lx ", &end, &next_start) < 2)
+                               break;
+
+                       next_start = RTE_ALIGN_CEIL(next_start, alloc_size);
+                       end = RTE_ALIGN_CEIL(end, alloc_size) - 1;
+               } else
+                       end = UINT64_MAX;
+
+               if (start >= high_limit)
+                       break;
+               if (end < low_limit)
+                       continue;
+
+               start = RTE_MAX(start, low_limit);
+               end = RTE_MIN(end, high_limit - 1);
+
+               if (end - start >= alloc_size - 1) {
+                       addr = start;
+                       break;
+               }
+       } while (end != UINT64_MAX);
+
+       fclose(fp);
+
+       return addr;
+}
+
+/*
  * Try to mmap *size bytes in /dev/zero. If it is successful, return the
  * pointer to the mmap'd area and keep *size unmodified. Else, retry
  * with a smaller zone: decrease *size by hugepage_sz until it reaches
@@ -1126,6 +1195,25 @@ rte_eal_hugepage_init(void)
        /* get pointer to global configuration */
        mcfg = rte_eal_get_configuration()->mem_config;

+       if (internal_config.range_virtaddr_end) {
+               uint64_t pagesize = RTE_PGSIZE_4K;
+               struct hugepage_info *hpi;
+               unsigned n;
+               uintptr_t addr;
+
+               /* determine maximum hugepage size */
+               for (n = 0; n < internal_config.num_hugepage_sizes; n++) {
+                       hpi = &internal_config.hugepage_info[n];
+                       pagesize = RTE_MAX(hpi->hugepage_sz, pagesize);
+               }
+
+               addr = rte_eal_get_free_region(pagesize);
+               if (addr == 0)
+                       RTE_LOG(WARNING, EAL,
+                               "no free space to mmap in specified
region\n");
+               internal_config.base_virtaddr = addr;
+       }
+
        /* when hugetlbfs is disabled or single-file option is specified */
        if (internal_config.no_hugetlbfs || internal_config.single_file) {
                int fd;
@@ -1158,7 +1246,8 @@ rte_eal_hugepage_init(void)
                        return -1;
                }

-               addr = mmap(NULL, internal_config.memory,
+               addr = mmap((void *)internal_config.base_virtaddr,
+                           internal_config.memory,
                            PROT_READ | PROT_WRITE,
                            MAP_SHARED | MAP_POPULATE, fd, 0);
                if (addr == MAP_FAILED) {