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;
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Lu Baolu <baolu.lu@linux.intel.com>,
        Joerg Roedel <joro@8bytes.org>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Christoph Hellwig <hch@infradead.org>,
        "Raj, Ashok" <ashok.raj@intel.com>, Will Deacon <will@kernel.org>,
        "Robin Murphy" <robin.murphy@arm.com>,
        Jean-Philippe Brucker <jean-philippe@linaro.com>,
        "Jiang, Dave" <dave.jiang@intel.com>, Vinod Koul <vkoul@kernel.org>
CC:     Eric Auger <eric.auger@redhat.com>,
        "Liu, Yi L" <yi.l.liu@intel.com>,
        "Pan, Jacob jun" <jacob.jun.pan@intel.com>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Jean-Philippe Brucker" <jean-philippe@linaro.org>
Subject: RE: [PATCH v9 04/11] iommu: Add sva iommu_domain support
Thread-Topic: [PATCH v9 04/11] iommu: Add sva iommu_domain support
Thread-Index: AQHYhX35xSpIBtEUJk2c4sDKhBGbeq1i8Q6w
Date:   Mon, 27 Jun 2022 08:29:21 +0000
Message-ID: <BN9PR11MB5276FA1A1A8C20786D958C048CB99@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20220621144353.17547-1-baolu.lu@linux.intel.com>
 <20220621144353.17547-5-baolu.lu@linux.intel.com>
In-Reply-To: <20220621144353.17547-5-baolu.lu@linux.intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?SoR9V7ZJuQhcunk/D6D3GKy/YXlkm+sLeLoRw8QJSkmqFdH2qdwtNsYJenDz?=
 =?us-ascii?Q?kfrXYIlRn/kg6P2VR+hsgCYTgDA+GxMnNjSDizW3oILDYKypkTCFAjMXCxzT?=
 =?us-ascii?Q?y2uzX0eeUxyp2vNyyWX1tQ0NNSaHgQc3lOiOrSYFCLG5htmTuXBmkyhNmHjK?=
 =?us-ascii?Q?4FRW10R/GekvxRhT1GqZ83dqzAOJLsUMHqoqg2R5ew5JDS5BLOMecddPE+Xa?=
 =?us-ascii?Q?UrgEAonBP5QM/le8rCIuILZHdlKDPwkQwE/euiYCQHq0JH5Q44RaazNXuChz?=
 =?us-ascii?Q?eS/bcWSds85/wSWSbef2bmZAf7uUJxs6rDzZbmynkhLQkrT67l9bBOKE0g9n?=
 =?us-ascii?Q?Z24MFpnduDkfTH9fCdYWmMtrO/bEc6hGqEckuc85wBSflGP4/rMZW2Zvw+Op?=
 =?us-ascii?Q?NDGLAHnWhE6EiR0xaP99pdzlcs4ENiz8bi3gU52DWi/L3Wyl+qifaN5kgZbW?=
 =?us-ascii?Q?/2v5RG7+UHmCKWfz0YlNCxxiHOr0P7GOOmrBZbC8zJ0ghgBUj6e6dabHYfh+?=
 =?us-ascii?Q?jGIPaZSS5WyQkwhZFJytTbCcMCk0OecLjEhRCOeLHf+08JYsS395CrJ77KG/?=
 =?us-ascii?Q?WtX7rnLxe0Fj0zuS+rewKODRildTK0ImB/B599Uyym9OO8xY/PeGXArCtMCB?=
 =?us-ascii?Q?4C7SPs10EAGHeOugYLyNmjdePG/szv042TVibUslOdAZmSqPtNkUCd9R24R5?=
 =?us-ascii?Q?8uldd/hdL4NbZZ9t1tVd8y5J0+ZWHBx7i1Qdvpuuc09ls/pFWAlFj+fbRhlC?=
 =?us-ascii?Q?uHwqKPuKHQqZHG712PfN0DPug+W+MFoJSa5+EPM6ZgY9Voz/D1VV4GFyx9nr?=
 =?us-ascii?Q?Kyph8MOFm1VOqrG5RaOyMZ7NYUam/yJCs0WxXy42BIDZ0wtbl2GoiaGBcJDk?=
 =?us-ascii?Q?Ghaa1hFy3mXdvZ4GNbUONi62SjGhYKJZe/IkZ143YkbNWEmAvhWhpfbp7TGl?=
 =?us-ascii?Q?IUwiYMfBoT43Iani3LHd8GXBhWJxtw69iYuuM/DsrcUxmC3PAgG+6fqoPP4Q?=
 =?us-ascii?Q?3xPLG2m5sB3DvIRm1ieCrat/5rBz6hZedMNNR2FTO0I6X5Ywa+yNhyVXmNtq?=
 =?us-ascii?Q?yXvBMenIddlZ9b/7zoYrwTPRv55kvFm9OpZHzbOyHL7at9/3I0jm81fQZ3hQ?=
 =?us-ascii?Q?hFJjX3SLkGE9XkfmLsHpdmrGvFbYJjabsNRSOnlF+3bAUA5D33TdMm9o4IB2?=
 =?us-ascii?Q?HpJ9RBGt9fLhUtpi1iaOStaK2H6kdSRIqCae891P5ev1DbBNca6Ob+hXq+SW?=
 =?us-ascii?Q?XwBWqljU0Jsbwfj9Nneq/FMQAWMG2F9YI3YW/oV+qjB6ZirqQ9KMHQo6Sv7G?=
 =?us-ascii?Q?0frrr3oxTBknWUhPZ5bny+Y56No3R/MhINP+fKXH1OkXNkxkHVzcLmGed0B0?=
 =?us-ascii?Q?nt+Vwx7Peocx+oYgKTnVXDkUr1UMct+4jMD9fQGhxgMxEut9DBDZT7/Fa+BK?=
 =?us-ascii?Q?1Do0nXnNijRcQY2gLUfig7OlzT2YC9CRBA38JiCNu4b5eU3yFmEGEv2zHxcx?=
 =?us-ascii?Q?qr4k8xvaWxjTw4wnmvmupetDUtiNGrXEPBSk6t4HG1LJR4RYg0HNE3x2gRaV?=
 =?us-ascii?Q?lxqcbrSmMtTi4bk3rwzRhZ18IHwz2kxTmMceJ0wn?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f2492c1e-51f4-4264-74de-08da5817229f
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Jun 2022 08:29:21.4075
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 0G27U5uxOPxtcL6j5SsTfG5snDBV+H+Au0T4v+tOu7Ln/gfMIT5dIocdzvpWC65HcWtSmRrj0jsFVPFXXAbVtw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR11MB1834
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE 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

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 21, 2022 10:44 PM
>=20
> The sva iommu_domain represents a hardware pagetable that the IOMMU
> hardware could use for SVA translation. This adds some infrastructure
> to support SVA domain in the iommu common layer. It includes:
>=20
> - Extend the iommu_domain to support a new IOMMU_DOMAIN_SVA
> domain
>   type. The IOMMU drivers that support SVA should provide the sva
>   domain specific iommu_domain_ops.
> - Add a helper to allocate an SVA domain. The iommu_domain_free()
>   is still used to free an SVA domain.
> - Add helpers to attach an SVA domain to a device and the reverse
>   operation.
>=20
> Some buses, like PCI, route packets without considering the PASID value.
> Thus a DMA target address with PASID might be treated as P2P if the
> address falls into the MMIO BAR of other devices in the group. To make
> things simple, the attach/detach interfaces only apply to devices
> belonging to the singleton groups, and the singleton is immutable in
> fabric i.e. not affected by hotplug.
>=20
> The iommu_attach/detach_device_pasid() can be used for other purposes,
> such as kernel DMA with pasid, mediation device, etc.

I'd split this into two patches. One for adding iommu_attach/
detach_device_pasid() and set/block_dev_pasid ops, and the
other for adding SVA.

>  struct iommu_domain {
>  	unsigned type;
>  	const struct iommu_domain_ops *ops;
>  	unsigned long pgsize_bitmap;	/* Bitmap of page sizes in use */
> -	iommu_fault_handler_t handler;
> -	void *handler_token;
>  	struct iommu_domain_geometry geometry;
>  	struct iommu_dma_cookie *iova_cookie;
> +	union {
> +		struct {	/* IOMMU_DOMAIN_DMA */
> +			iommu_fault_handler_t handler;
> +			void *handler_token;
> +		};

why is it DMA domain specific? What about unmanaged=20
domain? Unrecoverable fault can happen on any type
including SVA. Hence I think above should be domain type
agnostic.

> +		struct {	/* IOMMU_DOMAIN_SVA */
> +			struct mm_struct *mm;
> +		};
> +	};
>  };
>=20


> +
> +struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
> +					    struct mm_struct *mm)
> +{
> +	const struct iommu_ops *ops =3D dev_iommu_ops(dev);
> +	struct iommu_domain *domain;
> +
> +	domain =3D ops->domain_alloc(IOMMU_DOMAIN_SVA);
> +	if (!domain)
> +		return NULL;
> +
> +	domain->type =3D IOMMU_DOMAIN_SVA;

It's a bit weird that the type has been specified when calling
ops->domain_alloc while it still leaves to the caller to set the
type. But this is not caused by this series. could be cleaned
up separately.

> +
> +	mutex_lock(&group->mutex);
> +	curr =3D xa_cmpxchg(&group->pasid_array, pasid, NULL, domain,
> GFP_KERNEL);
> +	if (curr)
> +		goto out_unlock;

Need check xa_is_err(old).