From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f66.google.com (mail-ed1-f66.google.com [209.85.208.66]) by dpdk.org (Postfix) with ESMTP id 9C6841B4F8 for ; Thu, 4 Oct 2018 19:41:14 +0200 (CEST) Received: by mail-ed1-f66.google.com with SMTP id q19-v6so9365874edr.1 for ; Thu, 04 Oct 2018 10:41:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=QL9WQpZB1425hjIn7g5MCRR6OMdfayHaunJmI2nECPI=; b=NUdusKAIsd1u0fgotnDSO9fI0SPnzH/oxwRgOcDjSr8J/mWbSx6jkwBD45kyzb0iFC RuhBE76PGtkhg7hN2mp68qVoY0DNI9EDWvkZgmfi89uX/b1FXQuZWxwwjXjVlr4AUgo+ 2DZUj7EyOyiGxY810+pnEqXG7xc8A0ci91HJFrepJUTYrqCfHMI6ILxwPdVEMxhGIgKW ladXUQ3L5Erz+wR61wn5WJefNuYBzCVaBIfKx7brbgud3iNyJ+CE5YI5/vW03AbG/rrj wVKwjwrufrSzRNU5bOh4qq9vox/2UHXzaG4U3BPQ+Eq1RUu5UuipxyXsrHzEJwJm5nGI 5jKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=QL9WQpZB1425hjIn7g5MCRR6OMdfayHaunJmI2nECPI=; b=SO9iPTjjKzlmRkccsaAdk7jkjo3wE5WPYFja0JbQ6h8lRV4keecKIMFA2XIqeiH1Wr gZAymIDb6EUMAwo+ibwPy/Cj9RwDMLj/aVddiHeQtcZdFy73qhIfs8QyyOj935uJ0XNY e514x22EPn0DG5CT2NE6l6zLr4SYM+fByf2fYwB4+NgbHnkSu13OYYs7P8UhZLr5XmzD ZzM4R5ILqkLs42zXyTxufAf5a2wJvkq64Wo9cL8M6cWcfK81WClGdXp+lqv9Ic2JLGQF dbYSsumjIUJ4jpKd/SyQpPMIbDRnIE2RUj/FLeeJ6CyPJi8s7dog9ZWMGOVv9rDORyYb k2lw== X-Gm-Message-State: ABuFfoh4x5/uxM7Z9bK30KuRK3oWJU4tDAXOZ2ROsvpIfvO67WrJnQ4g Lgxeu4ievERqwzZ5wf1L4yWGueLEC6qOV9wAGymTuyjM X-Google-Smtp-Source: ACcGV60HAVYzmpraTmKpBzoJqt2LW8uhNnep8pL/wcOYIjZsXbdnTljfN5NhC7liB7WXlWkIyGrvNmuqzrGTsJqaa3I= X-Received: by 2002:a50:f05e:: with SMTP id u30-v6mr9818682edl.91.1538674874174; Thu, 04 Oct 2018 10:41:14 -0700 (PDT) MIME-Version: 1.0 References: <1535719857-19092-1-git-send-email-alejandro.lucero@netronome.com> <1535719857-19092-2-git-send-email-alejandro.lucero@netronome.com> <1ec4866d-24d4-1881-dfbe-ca2ff878e8c9@intel.com> In-Reply-To: From: Alejandro Lucero Date: Thu, 4 Oct 2018 18:41:03 +0100 Message-ID: To: "Burakov, Anatoly" Cc: dev Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] Fwd: [PATCH v2 1/5] mem: add function for checking memsegs IOVAs addresses X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Oct 2018 17:41:14 -0000 On Thu, Oct 4, 2018 at 4:39 PM Burakov, Anatoly wrote: > On 04-Oct-18 1:59 PM, Alejandro Lucero wrote: > > I sent this email only to Anatoly. Sending it again to mailing list. > > > > On Wed, Oct 3, 2018 at 1:43 PM Burakov, Anatoly < > anatoly.burakov@intel.com> > > wrote: > > > >> On 31-Aug-18 1:50 PM, Alejandro Lucero wrote: > >>> A device can suffer addressing limitations. This functions checks > >>> memsegs have iovas within the supported range based on dma mask. > >>> > >>> PMD should use this during initialization if supported devices > >>> suffer addressing limitations, returning an error if this function > >>> returns memsegs out of range. > >>> > >>> Another potential usage is for emulated IOMMU hardware with addressing > >>> limitations. > >>> > >>> It is necessary to save the most restricted dma mask for checking > >>> memory allocated dynamically after initialization. > >>> > >>> Signed-off-by: Alejandro Lucero > >>> --- > >>> lib/librte_eal/common/eal_common_memory.c | 56 > >> +++++++++++++++++++++++ > >>> lib/librte_eal/common/include/rte_eal_memconfig.h | 3 ++ > >>> lib/librte_eal/common/include/rte_memory.h | 3 ++ > >>> lib/librte_eal/common/malloc_heap.c | 12 +++++ > >>> lib/librte_eal/linuxapp/eal/eal.c | 2 + > >>> lib/librte_eal/rte_eal_version.map | 1 + > >>> 6 files changed, 77 insertions(+) > >>> > >>> diff --git a/lib/librte_eal/common/eal_common_memory.c > >> b/lib/librte_eal/common/eal_common_memory.c > >>> index fbfb1b0..bdd8f44 100644 > >>> --- a/lib/librte_eal/common/eal_common_memory.c > >>> +++ b/lib/librte_eal/common/eal_common_memory.c > >>> @@ -383,6 +383,62 @@ struct virtiova { > >>> rte_memseg_walk(dump_memseg, f); > >>> } > >>> > >>> +static int > >>> +check_iova(const struct rte_memseg_list *msl __rte_unused, > >>> + const struct rte_memseg *ms, void *arg) > >>> +{ > >>> + uint64_t *mask = arg; > >>> + rte_iova_t iova; > >>> + > >>> + /* higher address within segment */ > >>> + iova = (ms->iova + ms->len) - 1; > >>> + if (!(iova & *mask)) > >>> + return 0; > >>> + > >>> + RTE_LOG(INFO, EAL, "memseg iova %"PRIx64", len %zx, out of > >> range\n", > >>> + ms->iova, ms->len); > >>> + > >>> + RTE_LOG(INFO, EAL, "\tusing dma mask %"PRIx64"\n", *mask); > >> > >> IMO putting these as INFO is overkill. I'd prefer not to spam the output > >> unless it's really important. Can this go under DEBUG? > >> > >> > > This checks comes from a device or from the alloc_pages_on_heap when > > expanding memory. If the check discovers an address out of mask, a device > > can not be used or the new memory can not be allocated. I think having > this > > info will help to understand why the device initialization or the memory > > allocation are failing. > > > > If this text is only displayed whenever there's an error, the log output > should be ERR, not INFO. If the error may or may not happen depending on > who called this function, then this information is not important enough > to display to the user (it should be displayed in the error handler of > the caller), and DEBUG should suffice. > > Ok. Makes sense. I will change it. Thanks > > > >> Also, the message is misleading. You stop before you have a chance to > >> check other masks, which may restrict them even further. You're > >> outputting the message about using DMA mask XXX but this may not be the > >> final DMA mask. > >> > > > > Well, this is the first triggering, and it is enough for reporting the > > problem and avoiding the device or the new memory to be used. > > > > Note that the mask is per device, and for the memory allocation case, it > is > > the most restrictive dma mask. So there are no other masks to try. > > Fair enough. > > > > > > > > >> > >>> + /* Stop the walk and change mask */ > >>> + *mask = 0; > >>> + return 1; > > No need for out-of-band communication, _walk() function will return 1 if > walk was stopped prematurely. Just check return value of walk(). > > Yes, that's right. I will use the return from the walk function instead. Thanks > -- > Thanks, > Anatoly >