From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <drc@linux.vnet.ibm.com>, 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>
<f3fe2cef-70fa-3cac-73b3-64e21305de15@linux.vnet.ibm.com>
<03bace21-0a55-e88b-7f13-2a8764125547@intel.com>
<ddbedfbb-57ed-a357-66f2-8939f387d32d@linux.vnet.ibm.com>
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
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: <ddbedfbb-57ed-a357-66f2-8939f387d32d@linux.vnet.ibm.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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