From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D0273C433E0 for ; Wed, 20 Jan 2021 20:48:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 84072235FF for ; Wed, 20 Jan 2021 20:48:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732721AbhATUs1 (ORCPT ); Wed, 20 Jan 2021 15:48:27 -0500 Received: from mga17.intel.com ([192.55.52.151]:60530 "EHLO mga17.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731514AbhATUsU (ORCPT ); Wed, 20 Jan 2021 15:48:20 -0500 IronPort-SDR: 1rU1P64yCiKAZuw5iv+Lv31L30Myu6ihxmYy8K/abKr9cP4Hg1ggVa0m2emtK2y4Di7lsy/caL 1EYmG3b23nDQ== X-IronPort-AV: E=McAfee;i="6000,8403,9870"; a="158950365" X-IronPort-AV: E=Sophos;i="5.79,362,1602572400"; d="scan'208";a="158950365" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 12:47:28 -0800 IronPort-SDR: L/wl4gvZ7TdUoU8zWusXveudB72pqSFzK8Nfs/i5YpgujXfQRwJYLNxC7jdT2Q2FyyqoJIO0g8 +oA8cmMfCEeA== X-IronPort-AV: E=Sophos;i="5.79,362,1602572400"; d="scan'208";a="366509663" Received: from djiang5-mobl1.amr.corp.intel.com (HELO [10.254.121.244]) ([10.254.121.244]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 12:47:26 -0800 Subject: Re: [PATCH v9 05/10] uacce: Enable IOMMU_DEV_FEAT_IOPF To: Jean-Philippe Brucker , joro@8bytes.org, will@kernel.org Cc: vivek.gautam@arm.com, guohanjun@huawei.com, Zhou Wang , linux-acpi@vger.kernel.org, zhangfei.gao@linaro.org, lenb@kernel.org, devicetree@vger.kernel.org, Arnd Bergmann , eric.auger@redhat.com, vdumpa@nvidia.com, robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman , rjw@rjwysocki.net, shameerali.kolothum.thodi@huawei.com, iommu@lists.linux-foundation.org, sudeep.holla@arm.com, robin.murphy@arm.com, linux-accelerators@lists.ozlabs.org, baolu.lu@linux.intel.com, Dan Williams , "Pan, Jacob jun" References: <20210108145217.2254447-1-jean-philippe@linaro.org> <20210108145217.2254447-6-jean-philippe@linaro.org> From: Dave Jiang Message-ID: Date: Wed, 20 Jan 2021 13:47:25 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <20210108145217.2254447-6-jean-philippe@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On 1/8/2021 7:52 AM, Jean-Philippe Brucker wrote: > The IOPF (I/O Page Fault) feature is now enabled independently from the > SVA feature, because some IOPF implementations are device-specific and > do not require IOMMU support for PCIe PRI or Arm SMMU stall. > > Enable IOPF unconditionally when enabling SVA for now. In the future, if > a device driver implementing a uacce interface doesn't need IOPF > support, it will need to tell the uacce module, for example with a new > flag. > > Signed-off-by: Jean-Philippe Brucker > --- > Cc: Arnd Bergmann > Cc: Greg Kroah-Hartman > Cc: Zhangfei Gao > Cc: Zhou Wang > --- > drivers/misc/uacce/uacce.c | 32 +++++++++++++++++++++++++------- > 1 file changed, 25 insertions(+), 7 deletions(-) > > diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c > index d07af4edfcac..41ef1eb62a14 100644 > --- a/drivers/misc/uacce/uacce.c > +++ b/drivers/misc/uacce/uacce.c > @@ -385,6 +385,24 @@ static void uacce_release(struct device *dev) > kfree(uacce); > } > > +static unsigned int uacce_enable_sva(struct device *parent, unsigned int flags) > +{ > + if (!(flags & UACCE_DEV_SVA)) > + return flags; > + > + flags &= ~UACCE_DEV_SVA; > + > + if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF)) > + return flags; > + > + if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA)) { > + iommu_dev_disable_feature(parent, IOMMU_DEV_FEAT_IOPF); > + return flags; > + } Sorry to jump in a bit late on this and not specifically towards the intent of this patch. But I'd like to start a discussion on if we want to push the iommu dev feature enabling to the device driver itself rather than having UACCE control this? Maybe allow the device driver to manage the feature bits and UACCE only verify that they are enabled? 1. The device driver knows what platform it's on and what specific feature bits its devices supports. Maybe in the future if there are feature bits that's needed on one platform and not on another? 2. This allows the possibility of multiple uacce device registered to 1 pci dev, which for a device with asymmetric queues (Intel DSA/idxd driver) that is desirable feature. The current setup forces a single uacce device per pdev. If additional uacce devs are registered, the first removal of uacce device will disable the feature bit for the rest of the registered devices. With uacce managing the feature bit, it would need to add device context to the parent pdev and ref counting. It may be cleaner to just allow device driver to manage the feature bits and the driver should have all the information on when the feature needs to be turned on and off. - DaveJ > + > + return flags | UACCE_DEV_SVA; > +} > + > /** > * uacce_alloc() - alloc an accelerator > * @parent: pointer of uacce parent device > @@ -404,11 +422,7 @@ struct uacce_device *uacce_alloc(struct device *parent, > if (!uacce) > return ERR_PTR(-ENOMEM); > > - if (flags & UACCE_DEV_SVA) { > - ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA); > - if (ret) > - flags &= ~UACCE_DEV_SVA; > - } > + flags = uacce_enable_sva(parent, flags); > > uacce->parent = parent; > uacce->flags = flags; > @@ -432,8 +446,10 @@ struct uacce_device *uacce_alloc(struct device *parent, > return uacce; > > err_with_uacce: > - if (flags & UACCE_DEV_SVA) > + if (flags & UACCE_DEV_SVA) { > iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); > + iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF); > + } > kfree(uacce); > return ERR_PTR(ret); > } > @@ -487,8 +503,10 @@ void uacce_remove(struct uacce_device *uacce) > mutex_unlock(&uacce->queues_lock); > > /* disable sva now since no opened queues */ > - if (uacce->flags & UACCE_DEV_SVA) > + if (uacce->flags & UACCE_DEV_SVA) { > iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); > + iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF); > + } > > if (uacce->cdev) > cdev_device_del(uacce->cdev, &uacce->dev); From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 56D6DC433DB for ; Wed, 20 Jan 2021 20:47:35 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 82A05235FF for ; Wed, 20 Jan 2021 20:47:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 82A05235FF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 16D7784E3A; Wed, 20 Jan 2021 20:47:34 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cp1yo8VujsQF; Wed, 20 Jan 2021 20:47:33 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 15FED84CF4; Wed, 20 Jan 2021 20:47:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D9294C08A1; Wed, 20 Jan 2021 20:47:32 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id C938AC013A for ; Wed, 20 Jan 2021 20:47:31 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id B60D8203C8 for ; Wed, 20 Jan 2021 20:47:31 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id D5q3I1mTQVMO for ; Wed, 20 Jan 2021 20:47:29 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by silver.osuosl.org (Postfix) with ESMTPS id 87FE52036E for ; Wed, 20 Jan 2021 20:47:29 +0000 (UTC) IronPort-SDR: IL3eH7JIZZpeuDxaovirb/GQjhKz8D2DtdjAUyyWGypmA+XAcuXCu05s2Ca5kJ0N0SJvBpYoc0 9BLZYUAG+wCA== X-IronPort-AV: E=McAfee;i="6000,8403,9870"; a="179258413" X-IronPort-AV: E=Sophos;i="5.79,362,1602572400"; d="scan'208";a="179258413" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 12:47:28 -0800 IronPort-SDR: L/wl4gvZ7TdUoU8zWusXveudB72pqSFzK8Nfs/i5YpgujXfQRwJYLNxC7jdT2Q2FyyqoJIO0g8 +oA8cmMfCEeA== X-IronPort-AV: E=Sophos;i="5.79,362,1602572400"; d="scan'208";a="366509663" Received: from djiang5-mobl1.amr.corp.intel.com (HELO [10.254.121.244]) ([10.254.121.244]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 12:47:26 -0800 Subject: Re: [PATCH v9 05/10] uacce: Enable IOMMU_DEV_FEAT_IOPF To: Jean-Philippe Brucker , joro@8bytes.org, will@kernel.org References: <20210108145217.2254447-1-jean-philippe@linaro.org> <20210108145217.2254447-6-jean-philippe@linaro.org> From: Dave Jiang Message-ID: Date: Wed, 20 Jan 2021 13:47:25 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <20210108145217.2254447-6-jean-philippe@linaro.org> Content-Language: en-US Cc: vivek.gautam@arm.com, guohanjun@huawei.com, linux-acpi@vger.kernel.org, zhangfei.gao@linaro.org, lenb@kernel.org, devicetree@vger.kernel.org, Arnd Bergmann , robh+dt@kernel.org, Dan Williams , linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman , rjw@rjwysocki.net, iommu@lists.linux-foundation.org, "Pan, Jacob jun" , sudeep.holla@arm.com, robin.murphy@arm.com, linux-accelerators@lists.ozlabs.org X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" On 1/8/2021 7:52 AM, Jean-Philippe Brucker wrote: > The IOPF (I/O Page Fault) feature is now enabled independently from the > SVA feature, because some IOPF implementations are device-specific and > do not require IOMMU support for PCIe PRI or Arm SMMU stall. > > Enable IOPF unconditionally when enabling SVA for now. In the future, if > a device driver implementing a uacce interface doesn't need IOPF > support, it will need to tell the uacce module, for example with a new > flag. > > Signed-off-by: Jean-Philippe Brucker > --- > Cc: Arnd Bergmann > Cc: Greg Kroah-Hartman > Cc: Zhangfei Gao > Cc: Zhou Wang > --- > drivers/misc/uacce/uacce.c | 32 +++++++++++++++++++++++++------- > 1 file changed, 25 insertions(+), 7 deletions(-) > > diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c > index d07af4edfcac..41ef1eb62a14 100644 > --- a/drivers/misc/uacce/uacce.c > +++ b/drivers/misc/uacce/uacce.c > @@ -385,6 +385,24 @@ static void uacce_release(struct device *dev) > kfree(uacce); > } > > +static unsigned int uacce_enable_sva(struct device *parent, unsigned int flags) > +{ > + if (!(flags & UACCE_DEV_SVA)) > + return flags; > + > + flags &= ~UACCE_DEV_SVA; > + > + if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF)) > + return flags; > + > + if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA)) { > + iommu_dev_disable_feature(parent, IOMMU_DEV_FEAT_IOPF); > + return flags; > + } Sorry to jump in a bit late on this and not specifically towards the intent of this patch. But I'd like to start a discussion on if we want to push the iommu dev feature enabling to the device driver itself rather than having UACCE control this? Maybe allow the device driver to manage the feature bits and UACCE only verify that they are enabled? 1. The device driver knows what platform it's on and what specific feature bits its devices supports. Maybe in the future if there are feature bits that's needed on one platform and not on another? 2. This allows the possibility of multiple uacce device registered to 1 pci dev, which for a device with asymmetric queues (Intel DSA/idxd driver) that is desirable feature. The current setup forces a single uacce device per pdev. If additional uacce devs are registered, the first removal of uacce device will disable the feature bit for the rest of the registered devices. With uacce managing the feature bit, it would need to add device context to the parent pdev and ref counting. It may be cleaner to just allow device driver to manage the feature bits and the driver should have all the information on when the feature needs to be turned on and off. - DaveJ > + > + return flags | UACCE_DEV_SVA; > +} > + > /** > * uacce_alloc() - alloc an accelerator > * @parent: pointer of uacce parent device > @@ -404,11 +422,7 @@ struct uacce_device *uacce_alloc(struct device *parent, > if (!uacce) > return ERR_PTR(-ENOMEM); > > - if (flags & UACCE_DEV_SVA) { > - ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA); > - if (ret) > - flags &= ~UACCE_DEV_SVA; > - } > + flags = uacce_enable_sva(parent, flags); > > uacce->parent = parent; > uacce->flags = flags; > @@ -432,8 +446,10 @@ struct uacce_device *uacce_alloc(struct device *parent, > return uacce; > > err_with_uacce: > - if (flags & UACCE_DEV_SVA) > + if (flags & UACCE_DEV_SVA) { > iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); > + iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF); > + } > kfree(uacce); > return ERR_PTR(ret); > } > @@ -487,8 +503,10 @@ void uacce_remove(struct uacce_device *uacce) > mutex_unlock(&uacce->queues_lock); > > /* disable sva now since no opened queues */ > - if (uacce->flags & UACCE_DEV_SVA) > + if (uacce->flags & UACCE_DEV_SVA) { > iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); > + iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF); > + } > > if (uacce->cdev) > cdev_device_del(uacce->cdev, &uacce->dev); _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9088EC433DB for ; Wed, 20 Jan 2021 20:49:56 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 313C923600 for ; Wed, 20 Jan 2021 20:49:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 313C923600 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=V54fDW8CgrzBv2pz2NkSaa9OOi9u7jIf4bufp8hdn4M=; b=ccAYmySScFwwuF/qGn00dn4QM B5Antp0kjpCy1/mBL0s0z5lqoqJYokn543V0zm/m7sw9JsW39RUgs9+qNyf4l81tV8sDdWG6JMyCF lq8TL24Nskh76tptYRu6ny2vCjE60/23F/rNKZHfoGSgfeB6WF9AzuzJ/U4FzBV1oRgdf5N+nJ9gp T0+ECFqNIXM/ecs1YP24JciBCc75dTL2MHUNZcdxk/VDFM9dUo4av81DNOwhDQMQKAQRlnioWdX7Z UotRN3xoMkaz0YBfq/fTZTBKa/egzoL6rvnj4T5zD7U3Mqz+Kg/plqpLJ7072/ymCLrvCWNder1Km Jbm4oOuoQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l2KOB-0003gB-MA; Wed, 20 Jan 2021 20:47:39 +0000 Received: from mga18.intel.com ([134.134.136.126]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1l2KO5-0003dA-Sf for linux-arm-kernel@lists.infradead.org; Wed, 20 Jan 2021 20:47:35 +0000 IronPort-SDR: 3+7Ae2FL/YBFmG5GRFo4H6FHhx/Gn29KdSpbniKFxB8fiC6ggHq/6Z7gc9HT6pl51NIuHk9HqX 0joKiG5gwgMA== X-IronPort-AV: E=McAfee;i="6000,8403,9870"; a="166838084" X-IronPort-AV: E=Sophos;i="5.79,362,1602572400"; d="scan'208";a="166838084" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 12:47:28 -0800 IronPort-SDR: L/wl4gvZ7TdUoU8zWusXveudB72pqSFzK8Nfs/i5YpgujXfQRwJYLNxC7jdT2Q2FyyqoJIO0g8 +oA8cmMfCEeA== X-IronPort-AV: E=Sophos;i="5.79,362,1602572400"; d="scan'208";a="366509663" Received: from djiang5-mobl1.amr.corp.intel.com (HELO [10.254.121.244]) ([10.254.121.244]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 12:47:26 -0800 Subject: Re: [PATCH v9 05/10] uacce: Enable IOMMU_DEV_FEAT_IOPF To: Jean-Philippe Brucker , joro@8bytes.org, will@kernel.org References: <20210108145217.2254447-1-jean-philippe@linaro.org> <20210108145217.2254447-6-jean-philippe@linaro.org> From: Dave Jiang Message-ID: Date: Wed, 20 Jan 2021 13:47:25 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <20210108145217.2254447-6-jean-philippe@linaro.org> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210120_154734_118335_4993956B X-CRM114-Status: GOOD ( 27.96 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: vivek.gautam@arm.com, guohanjun@huawei.com, Zhou Wang , linux-acpi@vger.kernel.org, zhangfei.gao@linaro.org, lenb@kernel.org, devicetree@vger.kernel.org, Arnd Bergmann , eric.auger@redhat.com, robh+dt@kernel.org, Dan Williams , linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman , rjw@rjwysocki.net, shameerali.kolothum.thodi@huawei.com, iommu@lists.linux-foundation.org, "Pan, Jacob jun" , sudeep.holla@arm.com, robin.murphy@arm.com, linux-accelerators@lists.ozlabs.org, baolu.lu@linux.intel.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 1/8/2021 7:52 AM, Jean-Philippe Brucker wrote: > The IOPF (I/O Page Fault) feature is now enabled independently from the > SVA feature, because some IOPF implementations are device-specific and > do not require IOMMU support for PCIe PRI or Arm SMMU stall. > > Enable IOPF unconditionally when enabling SVA for now. In the future, if > a device driver implementing a uacce interface doesn't need IOPF > support, it will need to tell the uacce module, for example with a new > flag. > > Signed-off-by: Jean-Philippe Brucker > --- > Cc: Arnd Bergmann > Cc: Greg Kroah-Hartman > Cc: Zhangfei Gao > Cc: Zhou Wang > --- > drivers/misc/uacce/uacce.c | 32 +++++++++++++++++++++++++------- > 1 file changed, 25 insertions(+), 7 deletions(-) > > diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c > index d07af4edfcac..41ef1eb62a14 100644 > --- a/drivers/misc/uacce/uacce.c > +++ b/drivers/misc/uacce/uacce.c > @@ -385,6 +385,24 @@ static void uacce_release(struct device *dev) > kfree(uacce); > } > > +static unsigned int uacce_enable_sva(struct device *parent, unsigned int flags) > +{ > + if (!(flags & UACCE_DEV_SVA)) > + return flags; > + > + flags &= ~UACCE_DEV_SVA; > + > + if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF)) > + return flags; > + > + if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA)) { > + iommu_dev_disable_feature(parent, IOMMU_DEV_FEAT_IOPF); > + return flags; > + } Sorry to jump in a bit late on this and not specifically towards the intent of this patch. But I'd like to start a discussion on if we want to push the iommu dev feature enabling to the device driver itself rather than having UACCE control this? Maybe allow the device driver to manage the feature bits and UACCE only verify that they are enabled? 1. The device driver knows what platform it's on and what specific feature bits its devices supports. Maybe in the future if there are feature bits that's needed on one platform and not on another? 2. This allows the possibility of multiple uacce device registered to 1 pci dev, which for a device with asymmetric queues (Intel DSA/idxd driver) that is desirable feature. The current setup forces a single uacce device per pdev. If additional uacce devs are registered, the first removal of uacce device will disable the feature bit for the rest of the registered devices. With uacce managing the feature bit, it would need to add device context to the parent pdev and ref counting. It may be cleaner to just allow device driver to manage the feature bits and the driver should have all the information on when the feature needs to be turned on and off. - DaveJ > + > + return flags | UACCE_DEV_SVA; > +} > + > /** > * uacce_alloc() - alloc an accelerator > * @parent: pointer of uacce parent device > @@ -404,11 +422,7 @@ struct uacce_device *uacce_alloc(struct device *parent, > if (!uacce) > return ERR_PTR(-ENOMEM); > > - if (flags & UACCE_DEV_SVA) { > - ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA); > - if (ret) > - flags &= ~UACCE_DEV_SVA; > - } > + flags = uacce_enable_sva(parent, flags); > > uacce->parent = parent; > uacce->flags = flags; > @@ -432,8 +446,10 @@ struct uacce_device *uacce_alloc(struct device *parent, > return uacce; > > err_with_uacce: > - if (flags & UACCE_DEV_SVA) > + if (flags & UACCE_DEV_SVA) { > iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); > + iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF); > + } > kfree(uacce); > return ERR_PTR(ret); > } > @@ -487,8 +503,10 @@ void uacce_remove(struct uacce_device *uacce) > mutex_unlock(&uacce->queues_lock); > > /* disable sva now since no opened queues */ > - if (uacce->flags & UACCE_DEV_SVA) > + if (uacce->flags & UACCE_DEV_SVA) { > iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); > + iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF); > + } > > if (uacce->cdev) > cdev_device_del(uacce->cdev, &uacce->dev); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel