dev.dpdk.org archive mirror
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Rajesh Ravi <rajesh.ravi@broadcom.com>
Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>,
	dev@dpdk.org,
	Jonathan Richardson <jonathan.richardson@broadcom.com>,
	Scott Branden <scott.branden@broadcom.com>,
	Vikram Mysore Prakash <vikram.prakash@broadcom.com>,
	Srinath Mannam <srinath.mannam@broadcom.com>
Subject: Re: [dpdk-dev] [PATCH] eal: add option --iso-cmem for external custom memory
Date: Tue, 5 Nov 2019 11:41:10 +0000	[thread overview]
Message-ID: <3a954a3f-1fc5-47f0-64f2-a7c8e3fbf964@intel.com> (raw)
In-Reply-To: <32f3cd1a-08bb-e4bc-c22c-53453b936dd3@intel.com>

On 04-Nov-19 10:25 AM, Burakov, Anatoly wrote:
> On 30-Oct-19 7:50 PM, Rajesh Ravi wrote:
>> Thanks Anatoly.
>> Please find  inline below:
>>
>> [Anatoly] vfio_mem_event_callback() is called every time memory is 
>> added to a
>> heap. That includes internal and external memory
>>
>> [Rajesh] malloc_heap_add_external_memory() does call 
>> eal_memalloc_mem_event_notify [ instead of vfio_mem_event_callback() ]
>>                But, no callback function is getting called from inside 
>> eal_memalloc_mem_event_notify()
>>                execution flow is not entering inside following loop:
>>
>> /TAILQ_FOREACH(entry, &mem_event_callback_list, next) {/
>> /                 RTE_LOG(DEBUG, EAL, "Calling mem event callback 
>> '%s:%p'\n",
>>                           entry->name, entry->arg);
>>                   entry->clb(event, start, len, entry->arg);
>>                }/
>>
>> Do you mean to say,  we are supposed to explicitly register a callback 
>> which separately builds  iommu tables in addition to calling 
>> rte_malloc_heap_memory_add()  API?
> 
> Hi,
> 
> No, the callback in VFIO should be registered automatically [1] at EAL 
> initialization (or, more precisely, when default container is 
> initialized). Does that not happen in your case?
> 
> [1] http://git.dpdk.org/dpdk/tree/lib/librte_eal/linux/eal/eal_vfio.c#n791
> 

Hi Rajesh,

I think i figured it out. It is a defect in design of how external 
memory heaps are handled.

When VFIO initializes, it will find first VFIO-bound device, initialize 
the container, and set up DMA mappings. Then, you can add more memory 
through creating custom memory regions without adding them to heap 
(mmap() + rte_extmem_register() + rte_dev_dma_map()), or with adding 
them to heap (mmap() + rte_malloc_heap_add_memory()).

The problem is, memory registered through rte_dev_dma_map() will get 
added into a list of user maps, while heap memory will not - the 
assumption is that the DMA mapping will happen through the callback, but 
there is no record left anywhere that this memory is supposed to be mapped.

This makes it so that, if there are no VFIO-bound devices at startup, 
then you create a heap, and *then* you hotplug a device, the heap will 
not be mapped because (as you have correctly pointed out) type1_map() 
skips it, and it's not present in a list of user mem maps either, 
because it is heap memory, so EAL is supposed to handle it by itself and 
not through user map list.

There could be two fixes here. The easiest one is to just add another 
flag to the memseglist - that will work for 19.11, and that's what i 
intend on doing since we're breaking ABI anyway.

For older releases, a different approach would be required (i think 
scanning heaps is best we can do here) in order to keep the ABI and not 
introduce new stuff into rte_memseg_list.

I'll submit a patch shortly, it would be great if you could test it.

-- 
Thanks,
Anatoly

  reply	other threads:[~2019-11-05 11:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-15  5:30 [dpdk-dev] [PATCH] eal: add option --iso-cmem for external custom memory Ajit Khaparde
2019-10-17 15:45 ` Burakov, Anatoly
2019-10-18 10:54   ` Rajesh Ravi
2019-10-18 16:53     ` Burakov, Anatoly
2019-10-21 15:46       ` Rajesh Ravi
2019-10-22  7:56         ` Rajesh Ravi
2019-10-24 11:43           ` Burakov, Anatoly
2019-10-25 12:53             ` Rajesh Ravi
2019-10-25 15:02               ` Burakov, Anatoly
2019-10-30 19:50                 ` Rajesh Ravi
2019-11-04 10:25                   ` Burakov, Anatoly
2019-11-05 11:41                     ` Burakov, Anatoly [this message]
2019-11-05 14:10                       ` Rajesh Ravi
2019-11-05 15:18                         ` Burakov, Anatoly
2019-11-05 17:13                           ` Burakov, Anatoly
2019-11-06 13:55                             ` David Marchand
2019-11-07 15:51                               ` David Marchand
2019-11-07 16:16                                 ` Rajesh Ravi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3a954a3f-1fc5-47f0-64f2-a7c8e3fbf964@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=dev@dpdk.org \
    --cc=jonathan.richardson@broadcom.com \
    --cc=rajesh.ravi@broadcom.com \
    --cc=scott.branden@broadcom.com \
    --cc=srinath.mannam@broadcom.com \
    --cc=vikram.prakash@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).