Message ID | 20230112142218.725622-4-eperezma@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp3914082wrt; Thu, 12 Jan 2023 06:32:59 -0800 (PST) X-Google-Smtp-Source: AMrXdXsfO8Yw0yBOunvxpX3aouwORME5Z2E8MjPQHmUbIomIEBsTlrDNG1CvXC8e5uRTP1lhluOA X-Received: by 2002:a17:90a:66c1:b0:228:d42c:d5d6 with SMTP id z1-20020a17090a66c100b00228d42cd5d6mr6553739pjl.27.1673533979096; Thu, 12 Jan 2023 06:32:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1673533979; cv=none; d=google.com; s=arc-20160816; b=QUsJ0LiNEC0vu0xZU5IfATTI6EXjutm1PS6QsxpuuE7pCZ5vcMAeByMfAk+6PooWO6 Se68QfWt7IDZEZ6fo/XJpsdrsafGkxZnyjP6URlSfb2Xqm+CwNZ1jYcVY5NCn5Prtmhn j56SsTAYO1N9KQzbSHGSesIW0Or5pcMjbXeq3lX7x/ODFA719rYwnoYfjzeXs6kn1JBu 4fgL/AXv/A6YTOq5EBRIoJuLd9Nx41pFpiSq6gFIImW021o8jga0p2SHI6aOwmGGheBn ZgRqQQjEtRCrwHoISMM1M6XpQKMDvF9LsCWC1k+87Wjm/M4KXVyI8tgn/Q/tuV3LM6bc e2XQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=xS+WhDCpVAXfoHuOOZG8UbA19f02GKhB5mYWzIxAKis=; b=mrt1YpVoPS+K7+WjdikdrphDp14g+qR5qTzPZNWFuA5hEZ596DwgPz4fQjVRhKnXNJ wOkoaua4sjw/iSNeJ/zES7cghmybsiPbmmm0y2vRMpliJfMLKVgZY6+22l5gKLaWoie3 HPtpCCm5souRJMvHE19TejlIUvw6VXosZ35l46ztA9mK+JgvYe6CJ8QCCJpqZFpTOGGo SF/EYbC/HYH10gFM8bkV8uykzLpOivuT/qZjiJ5/j0CpMd74NVW9MlMNTWCIi+CD5EhA vOt1h183tv0RZTUf7gihBpqwPdko9jj8vpTeiwtIZ6Pgg3JjpeXcm9ThgP/XAuJyw2pC uryg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=YurhqiEP; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id rj14-20020a17090b3e8e00b0022692bdf6c0si873361pjb.49.2023.01.12.06.32.46; Thu, 12 Jan 2023 06:32:59 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=YurhqiEP; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235937AbjALOcR (ORCPT <rfc822;zhuangel570@gmail.com> + 99 others); Thu, 12 Jan 2023 09:32:17 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54368 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237137AbjALOb0 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 12 Jan 2023 09:31:26 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 84D0259300 for <linux-kernel@vger.kernel.org>; Thu, 12 Jan 2023 06:22:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673533352; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xS+WhDCpVAXfoHuOOZG8UbA19f02GKhB5mYWzIxAKis=; b=YurhqiEPS1qkTpy6H3O7DLRyIhjLkKXd7k5H6m/CcStTNwF8pFglFlhCK39yyoRLh4JDpm Snr9jwKkTFJwtq9tU1aZyy4DobraJpDiS0JarnHqDh1pQVSZXu1zlaFhUoklU/x1cVqGd+ Y2FDhvwdKpDXc3SLe2gPbkN5ZKU4Qtc= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-450-MofelG4rOpKRCjq502kgtQ-1; Thu, 12 Jan 2023 09:22:28 -0500 X-MC-Unique: MofelG4rOpKRCjq502kgtQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E1B4F2805599; Thu, 12 Jan 2023 14:22:27 +0000 (UTC) Received: from eperezma.remote.csb (unknown [10.39.192.72]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4975B2166B29; Thu, 12 Jan 2023 14:22:26 +0000 (UTC) From: =?utf-8?q?Eugenio_P=C3=A9rez?= <eperezma@redhat.com> To: mst@redhat.com, elic@nvidia.com Cc: linux-kernel@vger.kernel.org, parav@nvidia.com, lulu@redhat.com, jasowang@redhat.com, virtualization@lists.linux-foundation.org, sgarzare@redhat.com, si-wei.liu@oracle.com Subject: [RFC 3/3] vdpa/mlx5: take iommu_lock at dup_iotlb Date: Thu, 12 Jan 2023 15:22:18 +0100 Message-Id: <20230112142218.725622-4-eperezma@redhat.com> In-Reply-To: <20230112142218.725622-1-eperezma@redhat.com> References: <20230112142218.725622-1-eperezma@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1754827565488605868?= X-GMAIL-MSGID: =?utf-8?q?1754827565488605868?= |
Series |
MLX Adress Space ID + Control VirtQueue fixes
|
|
Commit Message
Eugenio Perez Martin
Jan. 12, 2023, 2:22 p.m. UTC
Both iommu changes and lookup are protected by mlx5_vdpa_net->reslock at
this moment, but:
* These iotlb changes / queries are not in the fast data path.
* reslock belongs to netdev, while dup_iotlb seems generic.
* It's located in a different file than the lock it needs to hold
Justifies the lock acquisition.
Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
drivers/vdpa/mlx5/core/mr.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
Comments
> From: Eugenio Pérez <eperezma@redhat.com> > Sent: Thursday, 12 January 2023 16:22 > To: mst@redhat.com; Eli Cohen <elic@nvidia.com> > Cc: linux-kernel@vger.kernel.org; Parav Pandit <parav@nvidia.com>; > lulu@redhat.com; jasowang@redhat.com; virtualization@lists.linux- > foundation.org; sgarzare@redhat.com; si-wei.liu@oracle.com > Subject: [RFC 3/3] vdpa/mlx5: take iommu_lock at dup_iotlb > > Both iommu changes and lookup are protected by mlx5_vdpa_net->reslock at > this moment, but: > * These iotlb changes / queries are not in the fast data path. > * reslock belongs to netdev, while dup_iotlb seems generic. > * It's located in a different file than the lock it needs to hold > > Justifies the lock acquisition. > Following this reasoning we should take the spinlock wherever we reference an iotlb. Question if it make sense that the iotlb could change while it is being referenced. Can you identify a specific case for this? > Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC > setting") > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > drivers/vdpa/mlx5/core/mr.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c > index 878ee94efa78..e9c8a7f8ee1d 100644 > --- a/drivers/vdpa/mlx5/core/mr.c > +++ b/drivers/vdpa/mlx5/core/mr.c > @@ -454,13 +454,15 @@ static int dup_iotlb(struct mlx5_vdpa_dev *mvdev, > struct vhost_iotlb *src) > { > struct vhost_iotlb_map *map; > u64 start = 0, last = ULLONG_MAX; > - int err; > + int err = 0; > + > + spin_lock(&mvdev->cvq.iommu_lock); > > vhost_iotlb_reset(mvdev->cvq.iotlb); > > if (!src) { > err = vhost_iotlb_add_range(mvdev->cvq.iotlb, start, last, > start, VHOST_ACCESS_RW); > - return err; > + goto out; > } > > for (map = vhost_iotlb_itree_first(src, start, last); map; > @@ -468,9 +470,12 @@ static int dup_iotlb(struct mlx5_vdpa_dev *mvdev, > struct vhost_iotlb *src) > err = vhost_iotlb_add_range(mvdev->cvq.iotlb, map->start, > map->last, > map->addr, map->perm); > if (err) > - return err; > + goto out; > } > - return 0; > + > +out: > + spin_unlock(&mvdev->cvq.iommu_lock); > + return err; > } > > static void prune_iotlb(struct mlx5_vdpa_dev *mvdev) > -- > 2.31.1
On Mon, Jan 16, 2023 at 8:13 AM Eli Cohen <elic@nvidia.com> wrote: > > > From: Eugenio Pérez <eperezma@redhat.com> > > Sent: Thursday, 12 January 2023 16:22 > > To: mst@redhat.com; Eli Cohen <elic@nvidia.com> > > Cc: linux-kernel@vger.kernel.org; Parav Pandit <parav@nvidia.com>; > > lulu@redhat.com; jasowang@redhat.com; virtualization@lists.linux- > > foundation.org; sgarzare@redhat.com; si-wei.liu@oracle.com > > Subject: [RFC 3/3] vdpa/mlx5: take iommu_lock at dup_iotlb > > > > Both iommu changes and lookup are protected by mlx5_vdpa_net->reslock at > > this moment, but: > > * These iotlb changes / queries are not in the fast data path. > > * reslock belongs to netdev, while dup_iotlb seems generic. > > * It's located in a different file than the lock it needs to hold > > > > Justifies the lock acquisition. > > > > Following this reasoning we should take the spinlock wherever we reference an iotlb. vring.c:iotlb_translate takes it. > Question if it make sense that the iotlb could change while it is being referenced. > Can you identify a specific case for this? > Not at this moment, because both are protected by mlx5_vdpa_net->reslock before access or change iotlb. So this would require changes to be exploitable, that's true. However, to take the lock is the expected usage for vringh, so future changes to either mlx or vringh could miss it. Thanks! > > Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC > > setting") > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > --- > > drivers/vdpa/mlx5/core/mr.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c > > index 878ee94efa78..e9c8a7f8ee1d 100644 > > --- a/drivers/vdpa/mlx5/core/mr.c > > +++ b/drivers/vdpa/mlx5/core/mr.c > > @@ -454,13 +454,15 @@ static int dup_iotlb(struct mlx5_vdpa_dev *mvdev, > > struct vhost_iotlb *src) > > { > > struct vhost_iotlb_map *map; > > u64 start = 0, last = ULLONG_MAX; > > - int err; > > + int err = 0; > > + > > + spin_lock(&mvdev->cvq.iommu_lock); > > > > vhost_iotlb_reset(mvdev->cvq.iotlb); > > > > if (!src) { > > err = vhost_iotlb_add_range(mvdev->cvq.iotlb, start, last, > > start, VHOST_ACCESS_RW); > > - return err; > > + goto out; > > } > > > > for (map = vhost_iotlb_itree_first(src, start, last); map; > > @@ -468,9 +470,12 @@ static int dup_iotlb(struct mlx5_vdpa_dev *mvdev, > > struct vhost_iotlb *src) > > err = vhost_iotlb_add_range(mvdev->cvq.iotlb, map->start, > > map->last, > > map->addr, map->perm); > > if (err) > > - return err; > > + goto out; > > } > > - return 0; > > + > > +out: > > + spin_unlock(&mvdev->cvq.iommu_lock); > > + return err; > > } > > > > static void prune_iotlb(struct mlx5_vdpa_dev *mvdev) > > -- > > 2.31.1 >
diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c index 878ee94efa78..e9c8a7f8ee1d 100644 --- a/drivers/vdpa/mlx5/core/mr.c +++ b/drivers/vdpa/mlx5/core/mr.c @@ -454,13 +454,15 @@ static int dup_iotlb(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *src) { struct vhost_iotlb_map *map; u64 start = 0, last = ULLONG_MAX; - int err; + int err = 0; + + spin_lock(&mvdev->cvq.iommu_lock); vhost_iotlb_reset(mvdev->cvq.iotlb); if (!src) { err = vhost_iotlb_add_range(mvdev->cvq.iotlb, start, last, start, VHOST_ACCESS_RW); - return err; + goto out; } for (map = vhost_iotlb_itree_first(src, start, last); map; @@ -468,9 +470,12 @@ static int dup_iotlb(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *src) err = vhost_iotlb_add_range(mvdev->cvq.iotlb, map->start, map->last, map->addr, map->perm); if (err) - return err; + goto out; } - return 0; + +out: + spin_unlock(&mvdev->cvq.iommu_lock); + return err; } static void prune_iotlb(struct mlx5_vdpa_dev *mvdev)