From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 05273A04DB
for <public@inbox.dpdk.org>; Thu, 15 Oct 2020 13:57:58 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id C0B931E537;
Thu, 15 Oct 2020 13:57:56 +0200 (CEST)
Received: from mail-io1-f67.google.com (mail-io1-f67.google.com
[209.85.166.67]) by dpdk.org (Postfix) with ESMTP id AC1FE1DE80;
Thu, 15 Oct 2020 13:57:52 +0200 (CEST)
Received: by mail-io1-f67.google.com with SMTP id m17so3959135ioo.1;
Thu, 15 Oct 2020 04:57:52 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
h=mime-version:references:in-reply-to:from:date:message-id:subject:to
:cc:content-transfer-encoding;
bh=Geg9rTExjdfsHxvuFW2OlMBLFAugM0ZYxpgXiP0gFXk=;
b=b1Vei7DdP55sh5rKKGA/1TXqkT7wPHkkz6VlAyjqvCYuOF1EzezsUdHooQ6k/TSlzj
u6+brU2a84IoDj2MI6AmG65Pn1xshBT+1V5dmgmtAvxboOac2gE/bUTKUBn/UvHkJO5Z
rDIC6S1aCODKQJEOeZVRoQ32sCIonS2mvEwMZpG6X31HKKZz7K68pPGH9PhgYORbIj/x
BLfQPKMeQWsOFn2Z0wXALeMjfm9hk3FaUTMOtPljOHZNM0CwdIa8olOFbr9lca+8bLUm
Vz1xP3ITEH+5E2R2qq2IGZBVIHCRaxea2u2EcYgySxyg7aSsur86ZiNHBZJGVeMJukYI
NnGA==
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:content-transfer-encoding;
bh=Geg9rTExjdfsHxvuFW2OlMBLFAugM0ZYxpgXiP0gFXk=;
b=KLKjlrx3HwDTf0eIWn+0+vne2yY+tgzS/mZtFI7HbN5IWV0kJ5JxcoFzjKc8WwSx0E
bbm0ZJQykUtQpyQLkLLNsyUYO2w2ZIwMtMEWEXwPB26EmT4kVj5EfZHMBYVFCOnk2pwn
1qM5lWktFPdFMKMIGaBFX2aqr181zXd3/kmPsY1xruZoUfVTK9ETZ0IWTqNhCbKsrGkm
5ciN8Cy3sFn8SfjesQL5cOsc5mpWT2d66j8W8nTrtpyG8ZTkdQi/CZiM5UDx/Jx8GDec
9sqCtiwZQjbPf+0ovfhOJck+Mq7NB7fwerSrL/KIJpo/DLI+2ruXFrhqcdKUEaEGyDBT
vajw==
X-Gm-Message-State: AOAM5304pWP29EYiHwfJiUlO65PCYtBreoyqwax8/GIFHZXYeC77Nz40
r0+rduTQr2OaIXsP9hwIkuGseQgRCOnrS3IDvsg=
X-Google-Smtp-Source: ABdhPJxXRh5GJSvxjAA4J6ZlYU+WeDbdAFNFtKFrrJWNfRebw2JDEVXTuuDR4NFvwGIw6wuMb9OYTYgdCSSAT7dOmUk=
X-Received: by 2002:a05:6638:974:: with SMTP id
o20mr3238844jaj.37.1602763070945;
Thu, 15 Oct 2020 04:57:50 -0700 (PDT)
MIME-Version: 1.0
References: <20201012081106.10610-1-ndabilpuram@marvell.com>
<20201012081106.10610-3-ndabilpuram@marvell.com>
<05afb7f5-96bf-dffd-15dd-2024586f7290@intel.com>
<20201015060914.GA32207@outlook.office365.com>
<bd079e7f-d4f9-769f-2a45-1e93668c9c9b@intel.com>
In-Reply-To: <bd079e7f-d4f9-769f-2a45-1e93668c9c9b@intel.com>
From: Nithin Dabilpuram <nithind1988@gmail.com>
Date: Thu, 15 Oct 2020 17:27:38 +0530
Message-ID: <CAMuDWKTwkQgumHnMjdrmQ+Ldow2p-6eZj+uRT=WpOYyMEo+-2w@mail.gmail.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Cc: Nithin Dabilpuram <ndabilpuram@marvell.com>,
Jerin Jacob <jerinj@marvell.com>, dev@dpdk.org, stable@dpdk.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [dpdk-stable] [dpdk-dev] [EXT] Re: [PATCH 2/2] vfio: fix
partial DMA unmapping for VFIO type1
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>
On Thu, Oct 15, 2020 at 3:31 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 15-Oct-20 7:09 AM, Nithin Dabilpuram wrote:
> > On Wed, Oct 14, 2020 at 04:07:10PM +0100, Burakov, Anatoly wrote:
> >> External Email
> >>
> >> ----------------------------------------------------------------------
> >> On 12-Oct-20 9:11 AM, Nithin Dabilpuram wrote:
> >>> Partial unmapping is not supported for VFIO IOMMU type1
> >>> by kernel. Though kernel gives return as zero, the unmapped size
> >>> returned will not be same as expected. So check for
> >>> returned unmap size and return error.
> >>>
> >>> For case of DMA map/unmap triggered by heap allocations,
> >>> maintain granularity of memseg page size so that heap
> >>> expansion and contraction does not have this issue.
> >>
> >> This is quite unfortunate, because there was a different bug that had =
to do
> >> with kernel having a very limited number of mappings available [1], as=
a
> >> result of which the page concatenation code was added.
> >>
> >> It should therefore be documented that the dma_entry_limit parameter s=
hould
> >> be adjusted should the user run out of the DMA entries.
> >>
> >> [1] https://urldefense.proofpoint.com/v2/url?u=3Dhttps-3A__lore.kernel=
.org_lkml_155414977872.12780.13728555131525362206.stgit-40gimli.home_T_&d=
=3DDwICaQ&c=3DnKjWec2b6R0mOyPaz7xtfQ&r=3DFZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapi=
fdDfc8SFQ&m=3D3GMg-634_cdUCY4WpQPwjzZ_S4ckuMHOnt2FxyyjXMk&s=3DTJLzppkaDS95V=
GyRHX2hzflQfb9XLK0OiOszSXoeXKk&e=3D
>
> <snip>
>
> >>> RTE_LOG(ERR, EAL, " cannot clear DMA remapping, =
error %i (%s)\n",
> >>> errno, strerror(errno));
> >>> return -1;
> >>> + } else if (dma_unmap.size !=3D len) {
> >>> + RTE_LOG(ERR, EAL, " unexpected size %"PRIu64" of=
DMA "
> >>> + "remapping cleared instead of %"PRIu64"\n=
",
> >>> + (uint64_t)dma_unmap.size, len);
> >>> + rte_errno =3D EIO;
> >>> + return -1;
> >>> }
> >>> }
> >>> @@ -1853,6 +1869,12 @@ container_dma_unmap(struct vfio_config *vfio_c=
fg, uint64_t vaddr, uint64_t iova,
> >>> /* we're partially unmapping a previously mapped region, =
so we
> >>> * need to split entry into two.
> >>> */
> >>> + if (!vfio_cfg->vfio_iommu_type->partial_unmap) {
> >>> + RTE_LOG(DEBUG, EAL, "DMA partial unmap unsupporte=
d\n");
> >>> + rte_errno =3D ENOTSUP;
> >>> + ret =3D -1;
> >>> + goto out;
> >>> + }
> >>
> >> How would we ever arrive here if we never do more than 1 page worth of
> >> memory anyway? I don't think this is needed.
> >
> > container_dma_unmap() is called by user via rte_vfio_container_dma_unma=
p()
> > and when he maps we don't split it as we don't about his memory.
> > So if he maps multiple pages and tries to unmap partially, then we shou=
ld fail.
>
> Should we map it in page granularity then, instead of adding this
> discrepancy between EAL and user mapping? I.e. instead of adding a
> workaround, how about we just do the same thing for user mem mappings?
>
In heap mapping's we map and unmap it at huge page granularity as we will a=
lways
maintain that.
But here I think we don't know if user's allocation is huge page or
collection of system
pages. Only thing we can do here is map it at system page granularity which
could waste entries if he say really is working with hugepages. Isn't ?
>
> --
> Thanks,
> Anatoly