From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7AF02A04B7; Wed, 14 Oct 2020 11:28:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 375B01DAAC; Wed, 14 Oct 2020 11:28:01 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id F32C31D905 for ; Wed, 14 Oct 2020 11:27:58 +0200 (CEST) IronPort-SDR: FW1k2WfYAGOau38PulfOPCwoIBc5L4rs9djprAPdN1H7v7FtwGMf7WCGgTzAu+sgFFFiAOBh3N vSlUAToqS4zA== X-IronPort-AV: E=McAfee;i="6000,8403,9773"; a="145383539" X-IronPort-AV: E=Sophos;i="5.77,374,1596524400"; d="scan'208";a="145383539" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Oct 2020 02:27:57 -0700 IronPort-SDR: xtYpAf6JnseKmhJj4N0u1D8sFy0CHudFd5/Btn8FipVlfivQBJkr1/iPhPus+F3EWOdKOwgwjx 0nzWir+wwh1A== X-IronPort-AV: E=Sophos;i="5.77,374,1596524400"; d="scan'208";a="520311080" Received: from cfitzp2-mobl1.ger.corp.intel.com (HELO [10.251.84.158]) ([10.251.84.158]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Oct 2020 02:27:56 -0700 To: David Christensen , david.marchand@redhat.com Cc: dev@dpdk.org References: <20200630213823.1583764-1-drc@linux.vnet.ibm.com> <20200810210707.745083-1-drc@linux.vnet.ibm.com> <20200810210707.745083-2-drc@linux.vnet.ibm.com> <2c830988-c4db-7bdc-50f3-3fa445a81673@intel.com> <03bace21-0a55-e88b-7f13-2a8764125547@intel.com> From: "Burakov, Anatoly" Message-ID: <038f8196-f5da-d19a-0f2d-673750c75e0d@intel.com> Date: Wed, 14 Oct 2020 10:27:54 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 1/1] vfio: modify spapr iommu support to use static window sizing 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 12-Oct-20 8:19 PM, David Christensen wrote: > \>>>> -vfio_spapr_unmap_walk(const struct rte_memseg_list *msl, >>>>> -        const struct rte_memseg *ms, void *arg) >>>>> +vfio_spapr_size_walk(const struct rte_memseg_list *msl, void *arg) >>>>>   { >>>>> -    int *vfio_container_fd = arg; >>>>> +    struct spapr_size_walk_param *param = arg; >>>>> +    uint64_t max = (uint64_t) msl->base_va + (uint64_t) msl->len; >>>>> -    /* skip external memory that isn't a heap */ >>>>> -    if (msl->external && !msl->heap) >>>>> -        return 0; >>>>> +    if (msl->external) { >>>>> +        param->external++; >>>>> +        if (!msl->heap) >>>>> +            return 0; >>>>> +    } >>>> >>>> It would be nice to have some comments in the code explaining what >>>> we're skipping and why. >>> >>> Reviewing this again, my inclination is to skip ALL external memory, >>> which by definition would seem to be outside of IOMMU control, so the >>> code would read: >>> >>>     if (msl->external) { >>>         param->external++; >>>         return 0; >>>     } >> >> The external memory can still be mapped for DMA with rte_dev_dma_map() >> API. The heap memory is meant to be mapped automatically by DPDK, >> while the non-heap memory (created with rte_extmem_register() API) is >> meant to be managed by the user and will be mapped using the >> user_mem_map functions in this file. > > So for my purpose of identifying the memory range qualified for IOMMU > protection, are you saying that external memory in the heap should be > included in the DMA window calculation?  Like this: > >         if (msl->external && !msl->heap) { >                 /* ignore user managed external memory */ >                 param->is_user_managed = true; >                 return 0; >         } > > Dave I would say so, yes. I would also double check the user mem map path to see if it makes sense with these changes, and correctly calculates the new window, should it be needed. -- Thanks, Anatoly