Message ID | 20230808074944.7825-1-tina.zhang@intel.com |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c44e:0:b0:3f2:4152:657d with SMTP id w14csp2333688vqr; Tue, 8 Aug 2023 12:03:31 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHcduc4tgot5DROXkuhCQTacjv1NyQF1xUCY8JZd6HR/LHRQq/E9KEhUAWHblF7hSCYH6gw X-Received: by 2002:a17:907:a041:b0:991:bf04:2047 with SMTP id gz1-20020a170907a04100b00991bf042047mr369258ejc.14.1691521411562; Tue, 08 Aug 2023 12:03:31 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691521411; cv=none; d=google.com; s=arc-20160816; b=HR+ARpRm602Kpspc1cTl+gjjxwaQGLd16JVStwhE+3JJVvljerY6fxZsk03Lu14KCl mxbIxv8DnupH1ZwEf0ZJtqj9IaKPWYWwb7SK8DhrOECFIV4NunKRPKuJH62uRIsCxLWK NC/S+W5nlMaJpuhabEmmtzqbJdBDUOxzFvK0/sSxUF0i1O0rgWcFDu8mbVBmEA78UXih WT0FFfKdcA13q+eYfe555Y2Y/cLTORyhLaexv1IJsWTh4G2ejofqP2vwd+wIY+HKgtmr 8dF1HZOP0JrjtOr+cnp9B5WSZeXYf+fR1fkGxJAz20fQuQYB9vkrKwS9Jd3Gw3zbCzmZ UJzg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from :dkim-signature; bh=1iTJ6DTolJ8w8wF04eCAG14C5L5j3Kxv/tHwewzAEwM=; fh=/lTHmTEAhz5c6hgBRAPnmJsxw5vDTKZT1TYa5ynR+LA=; b=B3OjJLwFjhlncaDmWCRwjBKOUzzHXJ9VwA3E8i/vobge1BBZD+vQN/6Bwpws7zUIiE id/1v/pEuz+0djmcJQfny1FAL1yyKj70KUCmKCnUayY9m99YoVltiuMPq7RkG71IdmKK M69lPYHgmtRIooWIu+UzJQIorstgqyoF00IAb1wImRi6RSOo/Q5JylNS3rnUKVxOcPXe wxTzOLmL90M7WjxIJInr1mDikVWjGckCWw0ONCB9sT7O2KLFe4Jwtmi3KeJ4cZm6k6Xx udsKygP1OavsYFLPcPfVtlgT5YMPM1z1Fj21RhYEWsqLSF8m8cEvTKMfsrG9w/Kt+nDg qDig== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=VL3+r+Jo; 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=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h21-20020a1709060f5500b0099cf58066efsi1223097ejj.828.2023.08.08.12.03.07; Tue, 08 Aug 2023 12:03:31 -0700 (PDT) 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=@intel.com header.s=Intel header.b=VL3+r+Jo; 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=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229790AbjHHQma (ORCPT <rfc822;aaronkmseo@gmail.com> + 99 others); Tue, 8 Aug 2023 12:42:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39260 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233001AbjHHQlT (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 8 Aug 2023 12:41:19 -0400 Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.100]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C4FC915CB5 for <linux-kernel@vger.kernel.org>; Tue, 8 Aug 2023 08:54:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1691510094; x=1723046094; h=from:to:cc:subject:date:message-id; bh=bZYKDkWN/NzViSoHo0Sz+w7bafCwSfAclgWR77m9hZM=; b=VL3+r+Joia9AEKj6g474X8WaPHVDN953ZisfX033av7/XysWTrw5kTzx 34U9/lOVkcKyO25E0u9q2utSlMSO6M/JwHK2sHRWmSe6X3p62DUfe2W/b mgAV0w4RNqmVLy98+kI8HTiBU9ETC1vuHH25jlIsIXouv/uX2LIk6v75u znpNEcV0sj9eh0PNDO6WAkBDAmpRJyFMxXBMhnprb7wgKMdYKYfn4EfMY PLc2LHIpZm0SBJj880IcSqvYvF0yMsBUxv/59AdELXo+MnudssRUoBXhd oHFPJF7Bc3GRYOFfdcGFpnNTTweJDSEnmhIu24jugbliRG/3tyqJqYpLe A==; X-IronPort-AV: E=McAfee;i="6600,9927,10795"; a="437078606" X-IronPort-AV: E=Sophos;i="6.01,263,1684825200"; d="scan'208";a="437078606" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Aug 2023 00:49:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10795"; a="681146258" X-IronPort-AV: E=Sophos;i="6.01,263,1684825200"; d="scan'208";a="681146258" Received: from kechen-optiplex-9020.bj.intel.com ([10.238.156.126]) by orsmga003.jf.intel.com with ESMTP; 08 Aug 2023 00:49:49 -0700 From: Tina Zhang <tina.zhang@intel.com> To: Jason Gunthorpe <jgg@ziepe.ca>, Kevin Tian <kevin.tian@intel.com>, Lu Baolu <baolu.lu@linux.intel.com>, Michael Shavit <mshavit@google.com> Cc: iommu@lists.linux.dev, linux-kernel@vger.kernel.org, Tina Zhang <tina.zhang@intel.com> Subject: [PATCH 0/5] Share sva domains with all devices bound to a mm Date: Tue, 8 Aug 2023 15:49:39 +0800 Message-Id: <20230808074944.7825-1-tina.zhang@intel.com> X-Mailer: git-send-email 2.17.1 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_BLOCKED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED 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: INBOX X-GMAIL-THRID: 1773688755652745629 X-GMAIL-MSGID: 1773688755652745629 |
Series |
Share sva domains with all devices bound to a mm
|
|
Message
Zhang, Tina
Aug. 8, 2023, 7:49 a.m. UTC
A sva domain's lifetime begins with binding a device to a mm and ends by releasing all the bound devices from that sva domain. Technically, there could be more than one sva domain identified by the mm PASID for the use of bound devices issuing DMA transactions. To support mm PASID 1:n with sva domains, each mm needs to keep both a reference list of allocated sva domains and the corresponding PASID. However, currently, mm struct only has one pasid field for sva usage, which is used to keep the info of an assigned PASID. That pasid field cannot provide sufficient info to build up the 1:n mapping between PASID and sva domains. This patch-set fills the gap by adding an mm_iommu field[1], whose type is mm_iommu_data struct, to replace the old pasid field. The introduced mm_iommu_data struct keeps info of both a reference list of sva domains and an assigned PASID. [1]: https://lore.kernel.org/linux-iommu/ZIBxPd1%2FJCAle6yP@nvidia.com/ The RFC version of this patch-set is here: https://lore.kernel.org/linux-iommu/20230707013441.365583-1-tina.zhang@intel.com/ Tina Zhang (5): iommu: Add mm_get_pasid() helper function iommu: Call helper function to get assigned pasid value mm: Add structure to keep sva information iommu: Support mm PASID 1:n with sva domains mm: Deprecate pasid field arch/x86/kernel/traps.c | 2 +- .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++--- drivers/iommu/intel/svm.c | 8 +-- drivers/iommu/iommu-sva.c | 50 ++++++++++++------- include/linux/iommu.h | 19 +++++-- include/linux/mm_types.h | 3 +- kernel/fork.c | 1 - mm/init-mm.c | 3 -- 8 files changed, 58 insertions(+), 40 deletions(-)
Comments
On 2023/8/8 15:49, Tina Zhang wrote: > A sva domain's lifetime begins with binding a device to a mm and ends > by releasing all the bound devices from that sva domain. Technically, > there could be more than one sva domain identified by the mm PASID for > the use of bound devices issuing DMA transactions. > > To support mm PASID 1:n with sva domains, each mm needs to keep both a > reference list of allocated sva domains and the corresponding PASID. > However, currently, mm struct only has one pasid field for sva usage, > which is used to keep the info of an assigned PASID. That pasid field > cannot provide sufficient info to build up the 1:n mapping between PASID > and sva domains. Is it more appropriate to have the same life cycle for sva domain and mm pasid? I feel that they represent the same thing, that is, the address space shared by mm to a device. Best regards, baolu > > This patch-set fills the gap by adding an mm_iommu field[1], whose type is > mm_iommu_data struct, to replace the old pasid field. The introduced > mm_iommu_data struct keeps info of both a reference list of sva domains > and an assigned PASID. > > > [1]: https://lore.kernel.org/linux-iommu/ZIBxPd1%2FJCAle6yP@nvidia.com/ > > > The RFC version of this patch-set is here: > https://lore.kernel.org/linux-iommu/20230707013441.365583-1-tina.zhang@intel.com/ > > Tina Zhang (5): > iommu: Add mm_get_pasid() helper function > iommu: Call helper function to get assigned pasid value > mm: Add structure to keep sva information > iommu: Support mm PASID 1:n with sva domains > mm: Deprecate pasid field > > arch/x86/kernel/traps.c | 2 +- > .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++--- > drivers/iommu/intel/svm.c | 8 +-- > drivers/iommu/iommu-sva.c | 50 ++++++++++++------- > include/linux/iommu.h | 19 +++++-- > include/linux/mm_types.h | 3 +- > kernel/fork.c | 1 - > mm/init-mm.c | 3 -- > 8 files changed, 58 insertions(+), 40 deletions(-) >
> From: Zhang, Tina <tina.zhang@intel.com> > Sent: Tuesday, August 8, 2023 3:50 PM > > A sva domain's lifetime begins with binding a device to a mm and ends > by releasing all the bound devices from that sva domain. Technically, > there could be more than one sva domain identified by the mm PASID for > the use of bound devices issuing DMA transactions. Could you elaborate it with some concrete examples which motivate this change? > > To support mm PASID 1:n with sva domains, each mm needs to keep both a > reference list of allocated sva domains and the corresponding PASID. > However, currently, mm struct only has one pasid field for sva usage, > which is used to keep the info of an assigned PASID. That pasid field > cannot provide sufficient info to build up the 1:n mapping between PASID > and sva domains. > > This patch-set fills the gap by adding an mm_iommu field[1], whose type is > mm_iommu_data struct, to replace the old pasid field. The introduced > mm_iommu_data struct keeps info of both a reference list of sva domains > and an assigned PASID. > > > [1]: https://lore.kernel.org/linux-iommu/ZIBxPd1%2FJCAle6yP@nvidia.com/ > > > The RFC version of this patch-set is here: > https://lore.kernel.org/linux-iommu/20230707013441.365583-1- > tina.zhang@intel.com/ > > Tina Zhang (5): > iommu: Add mm_get_pasid() helper function > iommu: Call helper function to get assigned pasid value > mm: Add structure to keep sva information > iommu: Support mm PASID 1:n with sva domains > mm: Deprecate pasid field > > arch/x86/kernel/traps.c | 2 +- > .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++--- > drivers/iommu/intel/svm.c | 8 +-- > drivers/iommu/iommu-sva.c | 50 ++++++++++++------- > include/linux/iommu.h | 19 +++++-- > include/linux/mm_types.h | 3 +- > kernel/fork.c | 1 - > mm/init-mm.c | 3 -- > 8 files changed, 58 insertions(+), 40 deletions(-) > > -- > 2.17.1
> From: Baolu Lu <baolu.lu@linux.intel.com> > Sent: Wednesday, August 9, 2023 8:18 AM > > On 2023/8/8 15:49, Tina Zhang wrote: > > A sva domain's lifetime begins with binding a device to a mm and ends > > by releasing all the bound devices from that sva domain. Technically, > > there could be more than one sva domain identified by the mm PASID for > > the use of bound devices issuing DMA transactions. > > > > To support mm PASID 1:n with sva domains, each mm needs to keep both > a > > reference list of allocated sva domains and the corresponding PASID. > > However, currently, mm struct only has one pasid field for sva usage, > > which is used to keep the info of an assigned PASID. That pasid field > > cannot provide sufficient info to build up the 1:n mapping between PASID > > and sva domains. > > Is it more appropriate to have the same life cycle for sva domain and mm > pasid? I feel that they represent the same thing, that is, the address > space shared by mm to a device. > iirc it's a simplification to free mm pasid at __mmdrop() otherwise the implementation is tricky, but I don't remember all the detail...
On 2023/8/9 17:44, Tian, Kevin wrote: >> From: Baolu Lu<baolu.lu@linux.intel.com> >> Sent: Wednesday, August 9, 2023 8:18 AM >> >> On 2023/8/8 15:49, Tina Zhang wrote: >>> A sva domain's lifetime begins with binding a device to a mm and ends >>> by releasing all the bound devices from that sva domain. Technically, >>> there could be more than one sva domain identified by the mm PASID for >>> the use of bound devices issuing DMA transactions. >>> >>> To support mm PASID 1:n with sva domains, each mm needs to keep both >> a >>> reference list of allocated sva domains and the corresponding PASID. >>> However, currently, mm struct only has one pasid field for sva usage, >>> which is used to keep the info of an assigned PASID. That pasid field >>> cannot provide sufficient info to build up the 1:n mapping between PASID >>> and sva domains. >> Is it more appropriate to have the same life cycle for sva domain and mm >> pasid? I feel that they represent the same thing, that is, the address >> space shared by mm to a device. >> > iirc it's a simplification to free mm pasid at __mmdrop() otherwise the > implementation is tricky, but I don't remember all the detail... Yeah, probably we could also free the sva domains in __mmdrop()? Remove the refcount for sva domain just like what we did for pasid (at the beginning we had refcount for each pasid...). Best regards, baolu
On Wed, Aug 09, 2023 at 08:18:18AM +0800, Baolu Lu wrote: > On 2023/8/8 15:49, Tina Zhang wrote: > > A sva domain's lifetime begins with binding a device to a mm and ends > > by releasing all the bound devices from that sva domain. Technically, > > there could be more than one sva domain identified by the mm PASID for > > the use of bound devices issuing DMA transactions. > > > > To support mm PASID 1:n with sva domains, each mm needs to keep both a > > reference list of allocated sva domains and the corresponding PASID. > > However, currently, mm struct only has one pasid field for sva usage, > > which is used to keep the info of an assigned PASID. That pasid field > > cannot provide sufficient info to build up the 1:n mapping between PASID > > and sva domains. > > Is it more appropriate to have the same life cycle for sva domain and mm > pasid? I feel that they represent the same thing, that is, the address > space shared by mm to a device. No! The iommu_domain and the PASID are totally seperate objects with their own lifecycles. The SVA domain should NEVER be tied to the mm enqcmd PASID. We might decide to free all the domains and keep the PASID around (can we even revoke the enqcmd pasid while the MM is alive?) Jason
On 2023/8/9 22:46, Jason Gunthorpe wrote: > On Wed, Aug 09, 2023 at 08:18:18AM +0800, Baolu Lu wrote: >> On 2023/8/8 15:49, Tina Zhang wrote: >>> A sva domain's lifetime begins with binding a device to a mm and ends >>> by releasing all the bound devices from that sva domain. Technically, >>> there could be more than one sva domain identified by the mm PASID for >>> the use of bound devices issuing DMA transactions. >>> >>> To support mm PASID 1:n with sva domains, each mm needs to keep both a >>> reference list of allocated sva domains and the corresponding PASID. >>> However, currently, mm struct only has one pasid field for sva usage, >>> which is used to keep the info of an assigned PASID. That pasid field >>> cannot provide sufficient info to build up the 1:n mapping between PASID >>> and sva domains. >> Is it more appropriate to have the same life cycle for sva domain and mm >> pasid? I feel that they represent the same thing, that is, the address >> space shared by mm to a device. > No! The iommu_domain and the PASID are totally seperate objects with > their own lifecycles. > > The SVA domain should NEVER be tied to the mm enqcmd PASID. Okay. Fair enough. > > We might decide to free all the domains and keep the PASID around (can > we even revoke the enqcmd pasid while the MM is alive?) We ever did this and was removed to make code simple. Best regards, baolu
Hi, On 8/9/23 17:41, Tian, Kevin wrote: >> From: Zhang, Tina <tina.zhang@intel.com> >> Sent: Tuesday, August 8, 2023 3:50 PM >> >> A sva domain's lifetime begins with binding a device to a mm and ends >> by releasing all the bound devices from that sva domain. Technically, >> there could be more than one sva domain identified by the mm PASID for >> the use of bound devices issuing DMA transactions. > > Could you elaborate it with some concrete examples which motivate > this change? The motivation is to remove the superfluous IOTLB invalidation in current VT-d driver. Currently, in VT-d driver, due to lacking shared sva domain info, in intel_flush_svm_range(), both iotlb and dev-tlb invalidation operations are performed per-device. However, difference devices could be behind one IOMMU (e.g., four devices are behind one IOMMU) and invoking iotlb per-device gives us more iotlb invalidation than necessary (4 iotlb invalidation instead of 1). This issue may give more performance impact when in a virtual machine guest, as currently we have one virtual VT-d for in front of those virtual devices. This patch fixes this issue by attaching shared sva domain information to mm, so that it can be utilized in the mm_notifier_ops callbacks. Regards, -Tina > >> >> To support mm PASID 1:n with sva domains, each mm needs to keep both a >> reference list of allocated sva domains and the corresponding PASID. >> However, currently, mm struct only has one pasid field for sva usage, >> which is used to keep the info of an assigned PASID. That pasid field >> cannot provide sufficient info to build up the 1:n mapping between PASID >> and sva domains. >> >> This patch-set fills the gap by adding an mm_iommu field[1], whose type is >> mm_iommu_data struct, to replace the old pasid field. The introduced >> mm_iommu_data struct keeps info of both a reference list of sva domains >> and an assigned PASID. >> >> >> [1]: https://lore.kernel.org/linux-iommu/ZIBxPd1%2FJCAle6yP@nvidia.com/ >> >> >> The RFC version of this patch-set is here: >> https://lore.kernel.org/linux-iommu/20230707013441.365583-1- >> tina.zhang@intel.com/ >> >> Tina Zhang (5): >> iommu: Add mm_get_pasid() helper function >> iommu: Call helper function to get assigned pasid value >> mm: Add structure to keep sva information >> iommu: Support mm PASID 1:n with sva domains >> mm: Deprecate pasid field >> >> arch/x86/kernel/traps.c | 2 +- >> .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++--- >> drivers/iommu/intel/svm.c | 8 +-- >> drivers/iommu/iommu-sva.c | 50 ++++++++++++------- >> include/linux/iommu.h | 19 +++++-- >> include/linux/mm_types.h | 3 +- >> kernel/fork.c | 1 - >> mm/init-mm.c | 3 -- >> 8 files changed, 58 insertions(+), 40 deletions(-) >> >> -- >> 2.17.1 >
> From: Baolu Lu <baolu.lu@linux.intel.com> > Sent: Thursday, August 10, 2023 9:23 AM > On 2023/8/9 22:46, Jason Gunthorpe wrote: > > > > We might decide to free all the domains and keep the PASID around (can > > we even revoke the enqcmd pasid while the MM is alive?) > > We ever did this and was removed to make code simple. > https://lore.kernel.org/lkml/87mto0ckpd.ffs@tglx/
> From: Zhang, Tina <tina.zhang@intel.com> > Sent: Thursday, August 10, 2023 9:32 AM > > Hi, > > On 8/9/23 17:41, Tian, Kevin wrote: > >> From: Zhang, Tina <tina.zhang@intel.com> > >> Sent: Tuesday, August 8, 2023 3:50 PM > >> > >> A sva domain's lifetime begins with binding a device to a mm and ends > >> by releasing all the bound devices from that sva domain. Technically, > >> there could be more than one sva domain identified by the mm PASID for > >> the use of bound devices issuing DMA transactions. > > > > Could you elaborate it with some concrete examples which motivate > > this change? > The motivation is to remove the superfluous IOTLB invalidation in > current VT-d driver. > > Currently, in VT-d driver, due to lacking shared sva domain info, in > intel_flush_svm_range(), both iotlb and dev-tlb invalidation operations > are performed per-device. However, difference devices could be behind > one IOMMU (e.g., four devices are behind one IOMMU) and invoking iotlb > per-device gives us more iotlb invalidation than necessary (4 iotlb > invalidation instead of 1). This issue may give more performance impact > when in a virtual machine guest, as currently we have one virtual VT-d > for in front of those virtual devices. > > > This patch fixes this issue by attaching shared sva domain information > to mm, so that it can be utilized in the mm_notifier_ops callbacks. > that is one of the motivations. e.g. another one as Jason suggested is to cleanup to decouple the common sva logic from enqcmd. Both should be mentioned in next version cover letter.
On Thu, Aug 10, 2023 at 07:49:11AM +0000, Tian, Kevin wrote: > > From: Zhang, Tina <tina.zhang@intel.com> > > Sent: Thursday, August 10, 2023 9:32 AM > > > > Hi, > > > > On 8/9/23 17:41, Tian, Kevin wrote: > > >> From: Zhang, Tina <tina.zhang@intel.com> > > >> Sent: Tuesday, August 8, 2023 3:50 PM > > >> > > >> A sva domain's lifetime begins with binding a device to a mm and ends > > >> by releasing all the bound devices from that sva domain. Technically, > > >> there could be more than one sva domain identified by the mm PASID for > > >> the use of bound devices issuing DMA transactions. > > > > > > Could you elaborate it with some concrete examples which motivate > > > this change? > > The motivation is to remove the superfluous IOTLB invalidation in > > current VT-d driver. > > > > Currently, in VT-d driver, due to lacking shared sva domain info, in > > intel_flush_svm_range(), both iotlb and dev-tlb invalidation operations > > are performed per-device. However, difference devices could be behind > > one IOMMU (e.g., four devices are behind one IOMMU) and invoking iotlb > > per-device gives us more iotlb invalidation than necessary (4 iotlb > > invalidation instead of 1). This issue may give more performance impact > > when in a virtual machine guest, as currently we have one virtual VT-d > > for in front of those virtual devices. > > > > > > This patch fixes this issue by attaching shared sva domain information > > to mm, so that it can be utilized in the mm_notifier_ops callbacks. > > > > that is one of the motivations. e.g. another one as Jason suggested > is to cleanup to decouple the common sva logic from enqcmd. Both > should be mentioned in next version cover letter. I also want to purge all the de-duplication and refcounting code around mm's and sva_binds from the drivers. Eg see the mess this makes of SMMUv3. Core code provides a single iommu_domain per-mm for SVA. Driver can rely on this optimization and does not need to de-duplicate. Single domain tracks all attachments. Driver can optimize using that information by de-duplicating (eg ASID invalidation vs ATC invalidation) After this we need to fix the domain allocation op to add a 'alloc_domain_sva(dev, mm_struct)' op so that the drivers can setup their SVA domains fully in a nice lock-safe environment. Jason
Hi, On 8/9/23 18:51, Baolu Lu wrote: > On 2023/8/9 17:44, Tian, Kevin wrote: >>> From: Baolu Lu<baolu.lu@linux.intel.com> >>> Sent: Wednesday, August 9, 2023 8:18 AM >>> >>> On 2023/8/8 15:49, Tina Zhang wrote: >>>> A sva domain's lifetime begins with binding a device to a mm and ends >>>> by releasing all the bound devices from that sva domain. Technically, >>>> there could be more than one sva domain identified by the mm PASID for >>>> the use of bound devices issuing DMA transactions. >>>> >>>> To support mm PASID 1:n with sva domains, each mm needs to keep both >>> a >>>> reference list of allocated sva domains and the corresponding PASID. >>>> However, currently, mm struct only has one pasid field for sva usage, >>>> which is used to keep the info of an assigned PASID. That pasid field >>>> cannot provide sufficient info to build up the 1:n mapping between >>>> PASID >>>> and sva domains. >>> Is it more appropriate to have the same life cycle for sva domain and mm >>> pasid? I feel that they represent the same thing, that is, the address >>> space shared by mm to a device. >>> >> iirc it's a simplification to free mm pasid at __mmdrop() otherwise the >> implementation is tricky, but I don't remember all the detail... > > Yeah, probably we could also free the sva domains in __mmdrop()? Remove > the refcount for sva domain just like what we did for pasid (at the > beginning we had refcount for each pasid...). For sva usage, mm->mm_count is increased in iommu_sva_domain_alloc(), and gets decreased when the domain has no users (which is checked in iommu_sva_unbind_device()). So, in a mm's life time, there could be multiple sva domains, though they are using the same PASID. I think it makes sense to mm. Because it makes no sense to keep a sva domain alive when no users are using it, even though the mm is alive. Regards, -Tina > > Best regards, > baolu
Hi, On 8/10/23 15:49, Tian, Kevin wrote: >> From: Zhang, Tina <tina.zhang@intel.com> >> Sent: Thursday, August 10, 2023 9:32 AM >> >> Hi, >> >> On 8/9/23 17:41, Tian, Kevin wrote: >>>> From: Zhang, Tina <tina.zhang@intel.com> >>>> Sent: Tuesday, August 8, 2023 3:50 PM >>>> >>>> A sva domain's lifetime begins with binding a device to a mm and ends >>>> by releasing all the bound devices from that sva domain. Technically, >>>> there could be more than one sva domain identified by the mm PASID for >>>> the use of bound devices issuing DMA transactions. >>> >>> Could you elaborate it with some concrete examples which motivate >>> this change? >> The motivation is to remove the superfluous IOTLB invalidation in >> current VT-d driver. >> >> Currently, in VT-d driver, due to lacking shared sva domain info, in >> intel_flush_svm_range(), both iotlb and dev-tlb invalidation operations >> are performed per-device. However, difference devices could be behind >> one IOMMU (e.g., four devices are behind one IOMMU) and invoking iotlb >> per-device gives us more iotlb invalidation than necessary (4 iotlb >> invalidation instead of 1). This issue may give more performance impact >> when in a virtual machine guest, as currently we have one virtual VT-d >> for in front of those virtual devices. >> >> >> This patch fixes this issue by attaching shared sva domain information >> to mm, so that it can be utilized in the mm_notifier_ops callbacks. >> > > that is one of the motivations. e.g. another one as Jason suggested > is to cleanup to decouple the common sva logic from enqcmd. Both > should be mentioned in next version cover letter. Right. Regards, -Tina
Hi, On 8/11/23 00:27, Jason Gunthorpe wrote: > On Thu, Aug 10, 2023 at 07:49:11AM +0000, Tian, Kevin wrote: >>> From: Zhang, Tina <tina.zhang@intel.com> >>> Sent: Thursday, August 10, 2023 9:32 AM >>> >>> Hi, >>> >>> On 8/9/23 17:41, Tian, Kevin wrote: >>>>> From: Zhang, Tina <tina.zhang@intel.com> >>>>> Sent: Tuesday, August 8, 2023 3:50 PM >>>>> >>>>> A sva domain's lifetime begins with binding a device to a mm and ends >>>>> by releasing all the bound devices from that sva domain. Technically, >>>>> there could be more than one sva domain identified by the mm PASID for >>>>> the use of bound devices issuing DMA transactions. >>>> >>>> Could you elaborate it with some concrete examples which motivate >>>> this change? >>> The motivation is to remove the superfluous IOTLB invalidation in >>> current VT-d driver. >>> >>> Currently, in VT-d driver, due to lacking shared sva domain info, in >>> intel_flush_svm_range(), both iotlb and dev-tlb invalidation operations >>> are performed per-device. However, difference devices could be behind >>> one IOMMU (e.g., four devices are behind one IOMMU) and invoking iotlb >>> per-device gives us more iotlb invalidation than necessary (4 iotlb >>> invalidation instead of 1). This issue may give more performance impact >>> when in a virtual machine guest, as currently we have one virtual VT-d >>> for in front of those virtual devices. >>> >>> >>> This patch fixes this issue by attaching shared sva domain information >>> to mm, so that it can be utilized in the mm_notifier_ops callbacks. >>> >> >> that is one of the motivations. e.g. another one as Jason suggested >> is to cleanup to decouple the common sva logic from enqcmd. Both >> should be mentioned in next version cover letter. > > I also want to purge all the de-duplication and refcounting code > around mm's and sva_binds from the drivers. Eg see the mess this makes > of SMMUv3. > > Core code provides a single iommu_domain per-mm for SVA. Driver can > rely on this optimization and does not need to de-duplicate. > > Single domain tracks all attachments. Driver can optimize using that > information by de-duplicating (eg ASID invalidation vs ATC > invalidation) > > After this we need to fix the domain allocation op to add a > 'alloc_domain_sva(dev, mm_struct)' op so that the drivers can setup > their SVA domains fully in a nice lock-safe environment. Agree. These can be added to the to-do list. Regards, -Tina > > Jason