All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
@ 2019-02-13  4:02 Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 1/9] iommu: Add APIs for multiple domains per device Lu Baolu
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:02 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu

Hi,

The Mediate Device is a framework for fine-grained physical device
sharing across the isolated domains. Currently the mdev framework
is designed to be independent of the platform IOMMU support. As the
result, the DMA isolation relies on the mdev parent device in a
vendor specific way.

There are several cases where a mediated device could be protected
and isolated by the platform IOMMU. For example, Intel vt-d rev3.0
[1] introduces a new translation mode called 'scalable mode', which
enables PASID-granular translations. The vt-d scalable mode is the
key ingredient for Scalable I/O Virtualization [2] [3] which allows
sharing a device in minimal possible granularity (ADI - Assignable
Device Interface).

A mediated device backed by an ADI could be protected and isolated
by the IOMMU since 1) the parent device supports tagging an unique
PASID to all DMA traffic out of the mediated device; and 2) the DMA
translation unit (IOMMU) supports the PASID granular translation.
We can apply IOMMU protection and isolation to this kind of devices
just as what we are doing with an assignable PCI device.

In order to distinguish the IOMMU-capable mediated devices from those
which still need to rely on parent devices, this patch set adds one
new member in struct mdev_device.

* iommu_device
  - This, if set, indicates that the mediated device could
    be fully isolated and protected by IOMMU via attaching
    an iommu domain to this device. If empty, it indicates
    using vendor defined isolation.

Below helpers are added to set and get above iommu device in mdev core
implementation.

* mdev_set/get_iommu_device(dev, iommu_device)
  - Set or get the iommu device which represents this mdev
    in IOMMU's device scope. Drivers don't need to set the
    iommu device if it uses vendor defined isolation.

The mdev parent device driver could opt-in that the mdev could be
fully isolated and protected by the IOMMU when the mdev is being
created by invoking mdev_set_iommu_device() in its @create().

In the vfio_iommu_type1_attach_group(), a domain allocated through
iommu_domain_alloc() will be attached to the mdev iommu device if
an iommu device has been set. Otherwise, the dummy external domain
will be used and all the DMA isolation and protection are routed to
parent driver as the result.

On IOMMU side, a basic requirement is allowing to attach multiple
domains to a PCI device if the device advertises the capability
and the IOMMU hardware supports finer granularity translations than
the normal PCI Source ID based translation.

As the result, a PCI device could work in two modes: normal mode
and auxiliary mode. In the normal mode, a pci device could be
isolated in the Source ID granularity; the pci device itself could
be assigned to a user application by attaching a single domain
to it. In the auxiliary mode, a pci device could be isolated in
finer granularity, hence subsets of the device could be assigned
to different user level application by attaching a different domain
to each subset.

Below APIs are introduced in iommu generic layer for aux-domain
purpose:

* iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
  - Check whether both IOMMU and device support IOMMU aux
    domain feature. Below aux-domain specific interfaces
    are available only after this returns true.

* iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_AUX)
  - Enable/disable device specific aux-domain feature.

* iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
  - Check whether the aux domain specific feature enabled or
    not.

* iommu_aux_attach_device(domain, dev)
  - Attaches @domain to @dev in the auxiliary mode. Multiple
    domains could be attached to a single device in the
    auxiliary mode with each domain representing an isolated
    address space for an assignable subset of the device.

* iommu_aux_detach_device(domain, dev)
  - Detach @domain which has been attached to @dev in the
    auxiliary mode.

* iommu_aux_get_pasid(domain, dev)
  - Return ID used for finer-granularity DMA translation.
    For the Intel Scalable IOV usage model, this will be
    a PASID. The device which supports Scalable IOV needs
    to write this ID to the device register so that DMA
    requests could be tagged with a right PASID prefix.

In order for the ease of discussion, sometimes we call "a domain in
auxiliary mode' or simply 'an auxiliary domain' when a domain is
attached to a device for finer granularity translations. But we need
to keep in mind that this doesn't mean there is a differnt domain
type. A same domain could be bound to a device for Source ID based
translation, and bound to another device for finer granularity
translation at the same time.

This patch series extends both IOMMU and vfio components to support
mdev device passing through when it could be isolated and protected
by the IOMMU units. The first part of this series (PATCH 1/09~6/09)
adds the interfaces and implementation of the multiple domains per
device. The second part (PATCH 7/09~9/09) adds the iommu device
attribute to each mdev, determines isolation type according to the
existence of an iommu device when attaching group in vfio type1 iommu
module, and attaches the domain to iommu aware mediated devices.

References:
[1] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
[2] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
[3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf

Best regards,
Lu Baolu

Change log:
  v5->v6:
  - Add a new API iommu_dev_feature_enabled() to check whether
    an IOMMU specific feature is enabled.
  - Rework the vt-d specific per device feature ops according
    to Joerg's comments. [https://lkml.org/lkml/2019/1/11/302].
  - PATCH 2/9 is added to move intel_iommu_enable_pasid() out
    of the scope of CONFIG_INTEL_IOMMU_SVM without functional
    changes.
  - All patches are rebased on top of vt-d branch of Joerg's
    iommu tree.

  v4->v5:
  - The iommu APIs have been updated with Joerg's proposal posted
    here https://www.spinics.net/lists/iommu/msg31874.html.
  - Some typos in commit message and comments have been fixed.
  - PATCH 3/8 was split from 4/8 to ease code review.
  - mdev->domain was removed and could bring back when there's a
    real consumer.
  - Other code review comments I received during v4 review period
    except the EXPORT_SYMBOL vs. EXPORT_SYMBOL_GPL in PATCH 6/8.
  - Rebase all patches to 5.0-rc1.

  v3->v4:
  - Use aux domain specific interfaces for domain attach and detach.
  - Rebase all patches to 4.20-rc1.

  v2->v3:
  - Remove domain type enum and use a pointer on mdev_device instead.
  - Add a generic interface for getting/setting per device iommu
    attributions. And use it for query aux domain capability, enable
    aux domain and disable aux domain purpose.
  - Reuse iommu_domain_get_attr() to retrieve the id in a aux domain.
  - We discussed the impact of the default domain implementation
    on reusing iommu_at(de)tach_device() interfaces. We agreed
    that reusing iommu_at(de)tach_device() interfaces is the right
    direction and we could tweak the code to remove the impact.
    https://www.spinics.net/lists/kvm/msg175285.html  
  - Removed the RFC tag since no objections received.
  - This patch has been submitted separately.
    https://www.spinics.net/lists/kvm/msg173936.html

  v1->v2:
  - Rewrite the patches with the concept of auxiliary domains.

Lu Baolu (9):
  iommu: Add APIs for multiple domains per device
  iommu/vt-d: Move enable pasid out of CONFIG_INTEL_IOMMU_SVM
  iommu/vt-d: Add per-device IOMMU feature ops entries
  iommu/vt-d: Move common code out of iommu_attch_device()
  iommu/vt-d: Aux-domain specific domain attach/detach
  iommu/vt-d: Return ID associated with an auxiliary domain
  vfio/mdev: Add iommu related member in mdev_device
  vfio/type1: Add domain at(de)taching group helpers
  vfio/type1: Handle different mdev isolation type

 drivers/iommu/intel-iommu.c      | 399 ++++++++++++++++++++++++++++---
 drivers/iommu/intel-svm.c        |  19 +-
 drivers/iommu/iommu.c            |  91 +++++++
 drivers/vfio/mdev/mdev_core.c    |  18 ++
 drivers/vfio/mdev/mdev_private.h |   1 +
 drivers/vfio/vfio_iommu_type1.c  | 132 ++++++++--
 include/linux/intel-iommu.h      |  13 +-
 include/linux/iommu.h            |  70 ++++++
 include/linux/mdev.h             |  14 ++
 9 files changed, 704 insertions(+), 53 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v6 1/9] iommu: Add APIs for multiple domains per device
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
@ 2019-02-13  4:02 ` Lu Baolu
  2019-02-13 11:55   ` Jean-Philippe Brucker
  2019-02-13  4:02 ` [PATCH v6 2/9] iommu/vt-d: Move enable pasid out of CONFIG_INTEL_IOMMU_SVM Lu Baolu
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:02 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

Sharing a physical PCI device in a finer-granularity way
is becoming a consensus in the industry. IOMMU vendors
are also engaging efforts to support such sharing as well
as possible. Among the efforts, the capability of support
finer-granularity DMA isolation is a common requirement
due to the security consideration. With finer-granularity
DMA isolation, all DMA requests out of or to a subset of
a physical PCI device can be protected by the IOMMU. As a
result, there is a request in software to attach multiple
domains to a physical PCI device. One example of such use
model is the Intel Scalable IOV [1] [2]. The Intel vt-d
3.0 spec [3] introduces the scalable mode which enables
PASID granularity DMA isolation.

This adds the APIs to support multiple domains per device.
In order to ease the discussions, we call it 'a domain in
auxiliary mode' or simply 'auxiliary domain' when multiple
domains are attached to a physical device.

The APIs include:

* iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
  - Check whether both IOMMU and device support IOMMU aux
    domain feature. Below aux-domain specific interfaces
    are available only after this returns true.

* iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_AUX)
  - Enable/disable device specific aux-domain feature.

* iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
  - Check whether the aux domain specific feature enabled or
    not.

* iommu_aux_attach_device(domain, dev)
  - Attaches @domain to @dev in the auxiliary mode. Multiple
    domains could be attached to a single device in the
    auxiliary mode with each domain representing an isolated
    address space for an assignable subset of the device.

* iommu_aux_detach_device(domain, dev)
  - Detach @domain which has been attached to @dev in the
    auxiliary mode.

* iommu_aux_get_pasid(domain, dev)
  - Return ID used for finer-granularity DMA translation.
    For the Intel Scalable IOV usage model, this will be
    a PASID. The device which supports Scalable IOV needs
    to write this ID to the device register so that DMA
    requests could be tagged with a right PASID prefix.

This has been updated with the latest proposal from Joerg
posted here [5].

Many people involved in discussions of this design.

Kevin Tian <kevin.tian@intel.com>
Liu Yi L <yi.l.liu@intel.com>
Ashok Raj <ashok.raj@intel.com>
Sanjay Kumar <sanjay.k.kumar@intel.com>
Jacob Pan <jacob.jun.pan@linux.intel.com>
Alex Williamson <alex.williamson@redhat.com>
Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Joerg Roedel <joro@8bytes.org>

and some discussions can be found here [4] [5].

[1] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
[2] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
[3] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
[4] https://lkml.org/lkml/2018/7/26/4
[5] https://www.spinics.net/lists/iommu/msg31874.html

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Suggested-by: Kevin Tian <kevin.tian@intel.com>
Suggested-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Suggested-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/iommu.c | 91 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h | 70 +++++++++++++++++++++++++++++++++
 2 files changed, 161 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3ed4db334341..d0b323e8357f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2033,3 +2033,94 @@ int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
+
+/*
+ * Per device IOMMU features.
+ */
+bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features feat)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->dev_has_feat)
+		return ops->dev_has_feat(dev, feat);
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(iommu_dev_has_feature);
+
+int iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->dev_enable_feat)
+		return ops->dev_enable_feat(dev, feat);
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(iommu_dev_enable_feature);
+
+int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->dev_disable_feat)
+		return ops->dev_disable_feat(dev, feat);
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(iommu_dev_disable_feature);
+
+bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features feat)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->dev_feat_enabled)
+		return ops->dev_feat_enabled(dev, feat);
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
+
+/*
+ * Aux-domain specific attach/detach.
+ *
+ * Only works if iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX) returns true.
+ * Also, as long as domains are attached to a device through this interface,
+ * any tries to call iommu_attach_device() should fail (iommu_detach_device()
+ * can't fail, so we fail on the tryint to re-attach). This should make us safe
+ * against a device being attached to a guest as a whole while there are still
+ * pasid users on it (aux and sva).
+ */
+int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
+{
+	int ret = -ENODEV;
+
+	if (domain->ops->aux_attach_dev)
+		ret = domain->ops->aux_attach_dev(domain, dev);
+
+	if (!ret)
+		trace_attach_device_to_domain(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
+
+void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
+{
+	if (domain->ops->aux_detach_dev) {
+		domain->ops->aux_detach_dev(domain, dev);
+		trace_detach_device_from_domain(dev);
+	}
+}
+EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
+
+int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
+{
+	int ret = -ENODEV;
+
+	if (domain->ops->aux_get_pasid)
+		ret = domain->ops->aux_get_pasid(domain, dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e90da6b6f3d1..831eb29b35c5 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -156,6 +156,11 @@ struct iommu_resv_region {
 	enum iommu_resv_type	type;
 };
 
+/* Per device IOMMU features */
+enum iommu_dev_features {
+	IOMMU_DEV_FEAT_AUX,	/* Aux-domain feature */
+};
+
 #ifdef CONFIG_IOMMU_API
 
 /**
@@ -183,6 +188,11 @@ struct iommu_resv_region {
  * @domain_window_enable: Configure and enable a particular window for a domain
  * @domain_window_disable: Disable a particular window for a domain
  * @of_xlate: add OF master IDs to iommu grouping
+ * @dev_has/enable/disable_feat: per device entries to check/enable/disable
+ *                               iommu specific features.
+ * @dev_feat_enabled: check enabled feature
+ * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
+ * @aux_get_pasid: get the pasid given an aux-domain
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  */
 struct iommu_ops {
@@ -226,6 +236,17 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	/* Per device IOMMU features */
+	bool (*dev_has_feat)(struct device *dev, enum iommu_dev_features f);
+	bool (*dev_feat_enabled)(struct device *dev, enum iommu_dev_features f);
+	int (*dev_enable_feat)(struct device *dev, enum iommu_dev_features f);
+	int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
+
+	/* Aux-domain specific attach/detach entries */
+	int (*aux_attach_dev)(struct iommu_domain *domain, struct device *dev);
+	void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev);
+	int (*aux_get_pasid)(struct iommu_domain *domain, struct device *dev);
+
 	unsigned long pgsize_bitmap;
 };
 
@@ -412,6 +433,14 @@ static inline void dev_iommu_fwspec_set(struct device *dev,
 int iommu_probe_device(struct device *dev);
 void iommu_release_device(struct device *dev);
 
+bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features f);
+int iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features f);
+int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features f);
+bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features f);
+int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev);
+void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev);
+int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev);
+
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
@@ -696,6 +725,47 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline bool
+iommu_dev_has_feature(struct device *dev, enum iommu_dev_features feat)
+{
+	return false;
+}
+
+static inline bool
+iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features feat)
+{
+	return false;
+}
+
+static inline int
+iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
+{
+	return -ENODEV;
+}
+
+static inline int
+iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat)
+{
+	return -ENODEV;
+}
+
+static inline int
+iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
+{
+	return -ENODEV;
+}
+
+static inline void
+iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
+{
+}
+
+static inline int
+iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 2/9] iommu/vt-d: Move enable pasid out of CONFIG_INTEL_IOMMU_SVM
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 1/9] iommu: Add APIs for multiple domains per device Lu Baolu
@ 2019-02-13  4:02 ` Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 3/9] iommu/vt-d: Add per-device IOMMU feature ops entries Lu Baolu
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:02 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

This moves intel_iommu_enable_pasid() out of the scope of
CONFIG_INTEL_IOMMU_SVM with more and more features requiring
pasid function.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 22 +++++++---------------
 drivers/iommu/intel-svm.c   | 19 ++++++++++++++++++-
 include/linux/intel-iommu.h |  2 +-
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index af23cfc2a05e..c4e0024c9736 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5295,8 +5295,7 @@ static void intel_iommu_put_resv_regions(struct device *dev,
 	}
 }
 
-#ifdef CONFIG_INTEL_IOMMU_SVM
-int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev)
+int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev)
 {
 	struct device_domain_info *info;
 	struct context_entry *context;
@@ -5305,7 +5304,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sd
 	u64 ctx_lo;
 	int ret;
 
-	domain = get_valid_domain_for_dev(sdev->dev);
+	domain = get_valid_domain_for_dev(dev);
 	if (!domain)
 		return -EINVAL;
 
@@ -5313,7 +5312,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sd
 	spin_lock(&iommu->lock);
 
 	ret = -EINVAL;
-	info = sdev->dev->archdata.iommu;
+	info = dev->archdata.iommu;
 	if (!info || !info->pasid_supported)
 		goto out;
 
@@ -5322,15 +5321,13 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sd
 		goto out;
 
 	ctx_lo = context[0].lo;
-
-	sdev->did = domain->iommu_did[iommu->seq_id];
-	sdev->sid = PCI_DEVID(info->bus, info->devfn);
-
 	if (!(ctx_lo & CONTEXT_PASIDE)) {
 		ctx_lo |= CONTEXT_PASIDE;
 		context[0].lo = ctx_lo;
 		wmb();
-		iommu->flush.flush_context(iommu, sdev->did, sdev->sid,
+		iommu->flush.flush_context(iommu,
+					   domain->iommu_did[iommu->seq_id],
+					   PCI_DEVID(info->bus, info->devfn),
 					   DMA_CCMD_MASK_NOBIT,
 					   DMA_CCMD_DEVICE_INVL);
 	}
@@ -5339,12 +5336,6 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sd
 	if (!info->pasid_enabled)
 		iommu_enable_dev_iotlb(info);
 
-	if (info->ats_enabled) {
-		sdev->dev_iotlb = 1;
-		sdev->qdep = info->ats_qdep;
-		if (sdev->qdep >= QI_DEV_EIOTLB_MAX_INVS)
-			sdev->qdep = 0;
-	}
 	ret = 0;
 
  out:
@@ -5354,6 +5345,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sd
 	return ret;
 }
 
+#ifdef CONFIG_INTEL_IOMMU_SVM
 struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
 {
 	struct intel_iommu *iommu;
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index c79540deaf00..ecc255ddf6ae 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -228,6 +228,7 @@ static LIST_HEAD(global_svm_list);
 int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ops *ops)
 {
 	struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
+	struct device_domain_info *info;
 	struct intel_svm_dev *sdev;
 	struct intel_svm *svm = NULL;
 	struct mm_struct *mm = NULL;
@@ -291,13 +292,29 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 	}
 	sdev->dev = dev;
 
-	ret = intel_iommu_enable_pasid(iommu, sdev);
+	ret = intel_iommu_enable_pasid(iommu, dev);
 	if (ret || !pasid) {
 		/* If they don't actually want to assign a PASID, this is
 		 * just an enabling check/preparation. */
 		kfree(sdev);
 		goto out;
 	}
+
+	info = dev->archdata.iommu;
+	if (!info || !info->pasid_supported) {
+		kfree(sdev);
+		goto out;
+	}
+
+	sdev->did = info->domain->iommu_did[iommu->seq_id];
+	sdev->sid = PCI_DEVID(info->bus, info->devfn);
+	if (info->ats_enabled) {
+		sdev->dev_iotlb = 1;
+		sdev->qdep = info->ats_qdep;
+		if (sdev->qdep >= QI_DEV_EIOTLB_MAX_INVS)
+			sdev->qdep = 0;
+	}
+
 	/* Finish the setup now we know we're keeping it */
 	sdev->users = 1;
 	sdev->ops = ops;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index fa364de9db18..b7d1e2fbb9ca 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -650,6 +650,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain *domain);
 int for_each_device_domain(int (*fn)(struct device_domain_info *info,
 				     void *data), void *data);
 void iommu_flush_write_buffer(struct intel_iommu *iommu);
+int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev);
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
 int intel_svm_init(struct intel_iommu *iommu);
@@ -679,7 +680,6 @@ struct intel_svm {
 	struct list_head list;
 };
 
-extern int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev);
 extern struct intel_iommu *intel_svm_device_to_iommu(struct device *dev);
 #endif
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 3/9] iommu/vt-d: Add per-device IOMMU feature ops entries
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 1/9] iommu: Add APIs for multiple domains per device Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 2/9] iommu/vt-d: Move enable pasid out of CONFIG_INTEL_IOMMU_SVM Lu Baolu
@ 2019-02-13  4:02 ` Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 4/9] iommu/vt-d: Move common code out of iommu_attch_device() Lu Baolu
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:02 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

This adds the iommu ops entries for aux-domain per-device
feature query and enable/disable.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Sanjay Kumar <sanjay.k.kumar@intel.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 159 ++++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h |   1 +
 2 files changed, 160 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c4e0024c9736..a7e2238f869a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2477,6 +2477,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 	info->domain = domain;
 	info->iommu = iommu;
 	info->pasid_table = NULL;
+	info->auxd_enabled = 0;
 
 	if (dev && dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(info->dev);
@@ -5211,6 +5212,42 @@ static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
 	return phys;
 }
 
+static inline bool scalable_mode_support(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	bool ret = true;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!sm_supported(iommu)) {
+			ret = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
+static inline bool iommu_pasid_support(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	bool ret = true;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!pasid_supported(iommu)) {
+			ret = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
 static bool intel_iommu_capable(enum iommu_cap cap)
 {
 	if (cap == IOMMU_CAP_CACHE_COHERENCY)
@@ -5367,6 +5404,124 @@ struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
 }
 #endif /* CONFIG_INTEL_IOMMU_SVM */
 
+static int intel_iommu_enable_auxd(struct device *dev)
+{
+	struct device_domain_info *info;
+	struct intel_iommu *iommu;
+	unsigned long flags;
+	u8 bus, devfn;
+	int ret;
+
+	iommu = device_to_iommu(dev, &bus, &devfn);
+	if (!iommu || dmar_disabled)
+		return -EINVAL;
+
+	if (!sm_supported(iommu) || !pasid_supported(iommu))
+		return -EINVAL;
+
+	ret = intel_iommu_enable_pasid(iommu, dev);
+	if (ret)
+		return -ENODEV;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	info = dev->archdata.iommu;
+	info->auxd_enabled = 1;
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	return 0;
+}
+
+static int intel_iommu_disable_auxd(struct device *dev)
+{
+	struct device_domain_info *info;
+	unsigned long flags;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	info = dev->archdata.iommu;
+	if (!WARN_ON(!info))
+		info->auxd_enabled = 0;
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	return 0;
+}
+
+/*
+ * A PCI express designated vendor specific extended capability is defined
+ * in the section 3.7 of Intel scalable I/O virtualization technical spec
+ * for system software and tools to detect endpoint devices supporting the
+ * Intel scalable IO virtualization without host driver dependency.
+ *
+ * Returns the address of the matching extended capability structure within
+ * the device's PCI configuration space or 0 if the device does not support
+ * it.
+ */
+static int siov_find_pci_dvsec(struct pci_dev *pdev)
+{
+	int pos;
+	u16 vendor, id;
+
+	pos = pci_find_next_ext_capability(pdev, 0, 0x23);
+	while (pos) {
+		pci_read_config_word(pdev, pos + 4, &vendor);
+		pci_read_config_word(pdev, pos + 8, &id);
+		if (vendor == PCI_VENDOR_ID_INTEL && id == 5)
+			return pos;
+
+		pos = pci_find_next_ext_capability(pdev, pos, 0x23);
+	}
+
+	return 0;
+}
+
+static bool
+intel_iommu_dev_has_feat(struct device *dev, enum iommu_dev_features feat)
+{
+	if (feat == IOMMU_DEV_FEAT_AUX) {
+		int ret;
+
+		if (!dev_is_pci(dev) || dmar_disabled ||
+		    !scalable_mode_support() || !iommu_pasid_support())
+			return false;
+
+		ret = pci_pasid_features(to_pci_dev(dev));
+		if (ret < 0)
+			return false;
+
+		return !!siov_find_pci_dvsec(to_pci_dev(dev));
+	}
+
+	return false;
+}
+
+static int
+intel_iommu_dev_enable_feat(struct device *dev, enum iommu_dev_features feat)
+{
+	if (feat == IOMMU_DEV_FEAT_AUX)
+		return intel_iommu_enable_auxd(dev);
+
+	return -ENODEV;
+}
+
+static int
+intel_iommu_dev_disable_feat(struct device *dev, enum iommu_dev_features feat)
+{
+	if (feat == IOMMU_DEV_FEAT_AUX)
+		return intel_iommu_disable_auxd(dev);
+
+	return -ENODEV;
+}
+
+static bool
+intel_iommu_dev_feat_enabled(struct device *dev, enum iommu_dev_features feat)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
+
+	if (feat == IOMMU_DEV_FEAT_AUX)
+		return scalable_mode_support() && info && info->auxd_enabled;
+
+	return false;
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -5381,6 +5536,10 @@ const struct iommu_ops intel_iommu_ops = {
 	.get_resv_regions	= intel_iommu_get_resv_regions,
 	.put_resv_regions	= intel_iommu_put_resv_regions,
 	.device_group		= pci_device_group,
+	.dev_has_feat		= intel_iommu_dev_has_feat,
+	.dev_feat_enabled	= intel_iommu_dev_feat_enabled,
+	.dev_enable_feat	= intel_iommu_dev_enable_feat,
+	.dev_disable_feat	= intel_iommu_dev_disable_feat,
 	.pgsize_bitmap		= INTEL_IOMMU_PGSIZES,
 };
 
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index b7d1e2fbb9ca..4f0745479b6d 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -568,6 +568,7 @@ struct device_domain_info {
 	u8 pri_enabled:1;
 	u8 ats_supported:1;
 	u8 ats_enabled:1;
+	u8 auxd_enabled:1;	/* Multiple domains per device */
 	u8 ats_qdep;
 	struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
 	struct intel_iommu *iommu; /* IOMMU used by this device */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 4/9] iommu/vt-d: Move common code out of iommu_attch_device()
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
                   ` (2 preceding siblings ...)
  2019-02-13  4:02 ` [PATCH v6 3/9] iommu/vt-d: Add per-device IOMMU feature ops entries Lu Baolu
@ 2019-02-13  4:02 ` Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 5/9] iommu/vt-d: Aux-domain specific domain attach/detach Lu Baolu
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:02 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

This part of code could be used by both normal and aux
domain specific attach entries. Hence move them into a
common function to avoid duplication.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 60 ++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index a7e2238f869a..3d83451a414d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5054,35 +5054,14 @@ static void intel_iommu_domain_free(struct iommu_domain *domain)
 	domain_exit(to_dmar_domain(domain));
 }
 
-static int intel_iommu_attach_device(struct iommu_domain *domain,
-				     struct device *dev)
+static int prepare_domain_attach_device(struct iommu_domain *domain,
+					struct device *dev)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	struct intel_iommu *iommu;
 	int addr_width;
 	u8 bus, devfn;
 
-	if (device_is_rmrr_locked(dev)) {
-		dev_warn(dev, "Device is ineligible for IOMMU domain attach due to platform RMRR requirement.  Contact your platform vendor.\n");
-		return -EPERM;
-	}
-
-	/* normally dev is not mapped */
-	if (unlikely(domain_context_mapped(dev))) {
-		struct dmar_domain *old_domain;
-
-		old_domain = find_domain(dev);
-		if (old_domain) {
-			rcu_read_lock();
-			dmar_remove_one_dev_info(dev);
-			rcu_read_unlock();
-
-			if (!domain_type_is_vm_or_si(old_domain) &&
-			     list_empty(&old_domain->devices))
-				domain_exit(old_domain);
-		}
-	}
-
 	iommu = device_to_iommu(dev, &bus, &devfn);
 	if (!iommu)
 		return -ENODEV;
@@ -5115,7 +5094,40 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 		dmar_domain->agaw--;
 	}
 
-	return domain_add_dev_info(dmar_domain, dev);
+	return 0;
+}
+
+static int intel_iommu_attach_device(struct iommu_domain *domain,
+				     struct device *dev)
+{
+	int ret;
+
+	if (device_is_rmrr_locked(dev)) {
+		dev_warn(dev, "Device is ineligible for IOMMU domain attach due to platform RMRR requirement.  Contact your platform vendor.\n");
+		return -EPERM;
+	}
+
+	/* normally dev is not mapped */
+	if (unlikely(domain_context_mapped(dev))) {
+		struct dmar_domain *old_domain;
+
+		old_domain = find_domain(dev);
+		if (old_domain) {
+			rcu_read_lock();
+			dmar_remove_one_dev_info(dev);
+			rcu_read_unlock();
+
+			if (!domain_type_is_vm_or_si(old_domain) &&
+			    list_empty(&old_domain->devices))
+				domain_exit(old_domain);
+		}
+	}
+
+	ret = prepare_domain_attach_device(domain, dev);
+	if (ret)
+		return ret;
+
+	return domain_add_dev_info(to_dmar_domain(domain), dev);
 }
 
 static void intel_iommu_detach_device(struct iommu_domain *domain,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 5/9] iommu/vt-d: Aux-domain specific domain attach/detach
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
                   ` (3 preceding siblings ...)
  2019-02-13  4:02 ` [PATCH v6 4/9] iommu/vt-d: Move common code out of iommu_attch_device() Lu Baolu
@ 2019-02-13  4:02 ` Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 6/9] iommu/vt-d: Return ID associated with an auxiliary domain Lu Baolu
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:02 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

When multiple domains per device has been enabled by the
device driver, the device will tag the default PASID for
the domain to all DMA traffics out of the subset of this
device; and the IOMMU should translate the DMA requests
in PASID granularity.

This adds the intel_iommu_aux_attach/detach_device() ops
to support managing PASID granular translation structures
when the device driver has enabled multiple domains per
device.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Sanjay Kumar <sanjay.k.kumar@intel.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 152 ++++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h |  10 +++
 2 files changed, 162 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3d83451a414d..8ecf09db6047 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2478,6 +2478,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 	info->iommu = iommu;
 	info->pasid_table = NULL;
 	info->auxd_enabled = 0;
+	INIT_LIST_HEAD(&info->auxiliary_domains);
 
 	if (dev && dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(info->dev);
@@ -5054,6 +5055,131 @@ static void intel_iommu_domain_free(struct iommu_domain *domain)
 	domain_exit(to_dmar_domain(domain));
 }
 
+/*
+ * Check whether a @domain could be attached to the @dev through the
+ * aux-domain attach/detach APIs.
+ */
+static inline bool
+is_aux_domain(struct device *dev, struct iommu_domain *domain)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
+
+	return info && info->auxd_enabled &&
+			domain->type == IOMMU_DOMAIN_UNMANAGED;
+}
+
+static void auxiliary_link_device(struct dmar_domain *domain,
+				  struct device *dev)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
+
+	assert_spin_locked(&device_domain_lock);
+	if (WARN_ON(!info))
+		return;
+
+	domain->auxd_refcnt++;
+	list_add(&domain->auxd, &info->auxiliary_domains);
+}
+
+static void auxiliary_unlink_device(struct dmar_domain *domain,
+				    struct device *dev)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
+
+	assert_spin_locked(&device_domain_lock);
+	if (WARN_ON(!info))
+		return;
+
+	list_del(&domain->auxd);
+	domain->auxd_refcnt--;
+
+	if (!domain->auxd_refcnt && domain->default_pasid > 0)
+		intel_pasid_free_id(domain->default_pasid);
+}
+
+static int aux_domain_add_dev(struct dmar_domain *domain,
+			      struct device *dev)
+{
+	int ret;
+	u8 bus, devfn;
+	unsigned long flags;
+	struct intel_iommu *iommu;
+
+	iommu = device_to_iommu(dev, &bus, &devfn);
+	if (!iommu)
+		return -ENODEV;
+
+	if (domain->default_pasid <= 0) {
+		int pasid;
+
+		pasid = intel_pasid_alloc_id(domain, PASID_MIN,
+					     pci_max_pasids(to_pci_dev(dev)),
+					     GFP_KERNEL);
+		if (pasid <= 0) {
+			pr_err("Can't allocate default pasid\n");
+			return -ENODEV;
+		}
+		domain->default_pasid = pasid;
+	}
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	/*
+	 * iommu->lock must be held to attach domain to iommu and setup the
+	 * pasid entry for second level translation.
+	 */
+	spin_lock(&iommu->lock);
+	ret = domain_attach_iommu(domain, iommu);
+	if (ret)
+		goto attach_failed;
+
+	/* Setup the PASID entry for mediated devices: */
+	ret = intel_pasid_setup_second_level(iommu, domain, dev,
+					     domain->default_pasid);
+	if (ret)
+		goto table_failed;
+	spin_unlock(&iommu->lock);
+
+	auxiliary_link_device(domain, dev);
+
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	return 0;
+
+table_failed:
+	domain_detach_iommu(domain, iommu);
+attach_failed:
+	spin_unlock(&iommu->lock);
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+	if (!domain->auxd_refcnt && domain->default_pasid > 0)
+		intel_pasid_free_id(domain->default_pasid);
+
+	return ret;
+}
+
+static void aux_domain_remove_dev(struct dmar_domain *domain,
+				  struct device *dev)
+{
+	struct device_domain_info *info;
+	struct intel_iommu *iommu;
+	unsigned long flags;
+
+	if (!is_aux_domain(dev, &domain->domain))
+		return;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	info = dev->archdata.iommu;
+	iommu = info->iommu;
+
+	auxiliary_unlink_device(domain, dev);
+
+	spin_lock(&iommu->lock);
+	intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid);
+	domain_detach_iommu(domain, iommu);
+	spin_unlock(&iommu->lock);
+
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+}
+
 static int prepare_domain_attach_device(struct iommu_domain *domain,
 					struct device *dev)
 {
@@ -5107,6 +5233,9 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 		return -EPERM;
 	}
 
+	if (is_aux_domain(dev, domain))
+		return -EPERM;
+
 	/* normally dev is not mapped */
 	if (unlikely(domain_context_mapped(dev))) {
 		struct dmar_domain *old_domain;
@@ -5130,12 +5259,33 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 	return domain_add_dev_info(to_dmar_domain(domain), dev);
 }
 
+static int intel_iommu_aux_attach_device(struct iommu_domain *domain,
+					 struct device *dev)
+{
+	int ret;
+
+	if (!is_aux_domain(dev, domain))
+		return -EPERM;
+
+	ret = prepare_domain_attach_device(domain, dev);
+	if (ret)
+		return ret;
+
+	return aux_domain_add_dev(to_dmar_domain(domain), dev);
+}
+
 static void intel_iommu_detach_device(struct iommu_domain *domain,
 				      struct device *dev)
 {
 	dmar_remove_one_dev_info(dev);
 }
 
+static void intel_iommu_aux_detach_device(struct iommu_domain *domain,
+					  struct device *dev)
+{
+	aux_domain_remove_dev(to_dmar_domain(domain), dev);
+}
+
 static int intel_iommu_map(struct iommu_domain *domain,
 			   unsigned long iova, phys_addr_t hpa,
 			   size_t size, int iommu_prot)
@@ -5540,6 +5690,8 @@ const struct iommu_ops intel_iommu_ops = {
 	.domain_free		= intel_iommu_domain_free,
 	.attach_dev		= intel_iommu_attach_device,
 	.detach_dev		= intel_iommu_detach_device,
+	.aux_attach_dev		= intel_iommu_aux_attach_device,
+	.aux_detach_dev		= intel_iommu_aux_detach_device,
 	.map			= intel_iommu_map,
 	.unmap			= intel_iommu_unmap,
 	.iova_to_phys		= intel_iommu_iova_to_phys,
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 4f0745479b6d..6925a18a5ca3 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -489,9 +489,11 @@ struct dmar_domain {
 					/* Domain ids per IOMMU. Use u16 since
 					 * domain ids are 16 bit wide according
 					 * to VT-d spec, section 9.3 */
+	unsigned int	auxd_refcnt;	/* Refcount of auxiliary attaching */
 
 	bool has_iotlb_device;
 	struct list_head devices;	/* all devices' list */
+	struct list_head auxd;		/* link to device's auxiliary list */
 	struct iova_domain iovad;	/* iova's that belong to this domain */
 
 	struct dma_pte	*pgd;		/* virtual address */
@@ -510,6 +512,11 @@ struct dmar_domain {
 					   2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
 	u64		max_addr;	/* maximum mapped address */
 
+	int		default_pasid;	/*
+					 * The default pasid used for non-SVM
+					 * traffic on mediated devices.
+					 */
+
 	struct iommu_domain domain;	/* generic domain data structure for
 					   iommu core */
 };
@@ -559,6 +566,9 @@ struct device_domain_info {
 	struct list_head link;	/* link to domain siblings */
 	struct list_head global; /* link to global list */
 	struct list_head table;	/* link to pasid table */
+	struct list_head auxiliary_domains; /* auxiliary domains
+					     * attached to this device
+					     */
 	u8 bus;			/* PCI bus number */
 	u8 devfn;		/* PCI devfn number */
 	u16 pfsid;		/* SRIOV physical function source ID */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 6/9] iommu/vt-d: Return ID associated with an auxiliary domain
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
                   ` (4 preceding siblings ...)
  2019-02-13  4:02 ` [PATCH v6 5/9] iommu/vt-d: Aux-domain specific domain attach/detach Lu Baolu
@ 2019-02-13  4:02 ` Lu Baolu
  2019-02-13  4:02 ` [PATCH v6 7/9] vfio/mdev: Add iommu related member in mdev_device Lu Baolu
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:02 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

This adds support to return the default pasid associated with
an auxiliary domain. The PCI device which is bound with this
domain should use this value as the pasid for all DMA requests
of the subset of device which is isolated and protected with
this domain.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Sanjay Kumar <sanjay.k.kumar@intel.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 8ecf09db6047..b79c72cc5931 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5684,6 +5684,15 @@ intel_iommu_dev_feat_enabled(struct device *dev, enum iommu_dev_features feat)
 	return false;
 }
 
+static int
+intel_iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
+{
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+
+	return dmar_domain->default_pasid > 0 ?
+			dmar_domain->default_pasid : -EINVAL;
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -5692,6 +5701,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.detach_dev		= intel_iommu_detach_device,
 	.aux_attach_dev		= intel_iommu_aux_attach_device,
 	.aux_detach_dev		= intel_iommu_aux_detach_device,
+	.aux_get_pasid		= intel_iommu_aux_get_pasid,
 	.map			= intel_iommu_map,
 	.unmap			= intel_iommu_unmap,
 	.iova_to_phys		= intel_iommu_iova_to_phys,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 7/9] vfio/mdev: Add iommu related member in mdev_device
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
                   ` (5 preceding siblings ...)
  2019-02-13  4:02 ` [PATCH v6 6/9] iommu/vt-d: Return ID associated with an auxiliary domain Lu Baolu
@ 2019-02-13  4:02 ` Lu Baolu
  2019-02-13  4:03 ` [PATCH v6 8/9] vfio/type1: Add domain at(de)taching group helpers Lu Baolu
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:02 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

A parent device might create different types of mediated
devices. For example, a mediated device could be created
by the parent device with full isolation and protection
provided by the IOMMU. One usage case could be found on
Intel platforms where a mediated device is an assignable
subset of a PCI, the DMA requests on behalf of it are all
tagged with a PASID. Since IOMMU supports PASID-granular
translations (scalable mode in VT-d 3.0), this mediated
device could be individually protected and isolated by an
IOMMU.

This patch adds a new member in the struct mdev_device to
indicate that the mediated device represented by mdev could
be isolated and protected by attaching a domain to a device
represented by mdev->iommu_device. It also adds a helper to
add or set the iommu device.

* mdev_device->iommu_device
  - This, if set, indicates that the mediated device could
    be fully isolated and protected by IOMMU via attaching
    an iommu domain to this device. If empty, it indicates
    using vendor defined isolation, hence bypass IOMMU.

* mdev_set/get_iommu_device(dev, iommu_device)
  - Set or get the iommu device which represents this mdev
    in IOMMU's device scope. Drivers don't need to set the
    iommu device if it uses vendor defined isolation.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Suggested-by: Kevin Tian <kevin.tian@intel.com>
Suggested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/vfio/mdev/mdev_core.c    | 18 ++++++++++++++++++
 drivers/vfio/mdev/mdev_private.h |  1 +
 include/linux/mdev.h             | 14 ++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 0212f0ee8aea..9be58d392d2b 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -390,6 +390,24 @@ int mdev_device_remove(struct device *dev, bool force_remove)
 	return 0;
 }
 
+int mdev_set_iommu_device(struct device *dev, struct device *iommu_device)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	mdev->iommu_device = iommu_device;
+
+	return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_device);
+
+struct device *mdev_get_iommu_device(struct device *dev)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	return mdev->iommu_device;
+}
+EXPORT_SYMBOL(mdev_get_iommu_device);
+
 static int __init mdev_init(void)
 {
 	return mdev_bus_register();
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
index b5819b7d7ef7..891841862ef8 100644
--- a/drivers/vfio/mdev/mdev_private.h
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -34,6 +34,7 @@ struct mdev_device {
 	struct list_head next;
 	struct kobject *type_kobj;
 	bool active;
+	struct device *iommu_device;
 };
 
 #define to_mdev_device(dev)	container_of(dev, struct mdev_device, dev)
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index b6e048e1045f..c3ab8a9cfcc7 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -15,6 +15,20 @@
 
 struct mdev_device;
 
+/*
+ * Called by the parent device driver to set the device which represents
+ * this mdev in iommu protection scope. By default, the iommu device is
+ * NULL, that indicates using vendor defined isolation.
+ *
+ * @dev: the mediated device that iommu will isolate.
+ * @iommu_device: a pci device which represents the iommu for @dev.
+ *
+ * Return 0 for success, otherwise negative error value.
+ */
+int mdev_set_iommu_device(struct device *dev, struct device *iommu_device);
+
+struct device *mdev_get_iommu_device(struct device *dev);
+
 /**
  * struct mdev_parent_ops - Structure to be registered for each parent device to
  * register the device to mdev module.
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 8/9] vfio/type1: Add domain at(de)taching group helpers
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
                   ` (6 preceding siblings ...)
  2019-02-13  4:02 ` [PATCH v6 7/9] vfio/mdev: Add iommu related member in mdev_device Lu Baolu
@ 2019-02-13  4:03 ` Lu Baolu
  2019-02-13  4:03 ` [PATCH v6 9/9] vfio/type1: Handle different mdev isolation type Lu Baolu
  2019-02-14 20:14 ` [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Alex Williamson
  9 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:03 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

This adds helpers to attach or detach a domain to a
group. This will replace iommu_attach_group() which
only works for non-mdev devices.

If a domain is attaching to a group which includes the
mediated devices, it should attach to the iommu device
(a pci device which represents the mdev in iommu scope)
instead. The added helper supports attaching domain to
groups for both pci and mdev devices.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Sanjay Kumar <sanjay.k.kumar@intel.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/vfio/vfio_iommu_type1.c | 84 ++++++++++++++++++++++++++++++---
 1 file changed, 77 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 73652e21efec..ccc4165474aa 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -91,6 +91,7 @@ struct vfio_dma {
 struct vfio_group {
 	struct iommu_group	*iommu_group;
 	struct list_head	next;
+	bool			mdev_group;	/* An mdev group */
 };
 
 /*
@@ -1298,6 +1299,75 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base)
 	return ret;
 }
 
+static struct device *vfio_mdev_get_iommu_device(struct device *dev)
+{
+	struct device *(*fn)(struct device *dev);
+	struct device *iommu_device;
+
+	fn = symbol_get(mdev_get_iommu_device);
+	if (fn) {
+		iommu_device = fn(dev);
+		symbol_put(mdev_get_iommu_device);
+
+		return iommu_device;
+	}
+
+	return NULL;
+}
+
+static int vfio_mdev_attach_domain(struct device *dev, void *data)
+{
+	struct iommu_domain *domain = data;
+	struct device *iommu_device;
+
+	iommu_device = vfio_mdev_get_iommu_device(dev);
+	if (iommu_device) {
+		if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
+			return iommu_aux_attach_device(domain, iommu_device);
+		else
+			return iommu_attach_device(domain, iommu_device);
+	}
+
+	return -EINVAL;
+}
+
+static int vfio_mdev_detach_domain(struct device *dev, void *data)
+{
+	struct iommu_domain *domain = data;
+	struct device *iommu_device;
+
+	iommu_device = vfio_mdev_get_iommu_device(dev);
+	if (iommu_device) {
+		if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
+			iommu_aux_detach_device(domain, iommu_device);
+		else
+			iommu_detach_device(domain, iommu_device);
+	}
+
+	return 0;
+}
+
+static int vfio_iommu_attach_group(struct vfio_domain *domain,
+				   struct vfio_group *group)
+{
+	if (group->mdev_group)
+		return iommu_group_for_each_dev(group->iommu_group,
+						domain->domain,
+						vfio_mdev_attach_domain);
+	else
+		return iommu_attach_group(domain->domain, group->iommu_group);
+}
+
+static void vfio_iommu_detach_group(struct vfio_domain *domain,
+				    struct vfio_group *group)
+{
+	if (group->mdev_group)
+		iommu_group_for_each_dev(group->iommu_group, domain->domain,
+					 vfio_mdev_detach_domain);
+	else
+		iommu_detach_group(domain->domain, group->iommu_group);
+}
+
 static int vfio_iommu_type1_attach_group(void *iommu_data,
 					 struct iommu_group *iommu_group)
 {
@@ -1373,7 +1443,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 			goto out_domain;
 	}
 
-	ret = iommu_attach_group(domain->domain, iommu_group);
+	ret = vfio_iommu_attach_group(domain, group);
 	if (ret)
 		goto out_domain;
 
@@ -1405,8 +1475,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	list_for_each_entry(d, &iommu->domain_list, next) {
 		if (d->domain->ops == domain->domain->ops &&
 		    d->prot == domain->prot) {
-			iommu_detach_group(domain->domain, iommu_group);
-			if (!iommu_attach_group(d->domain, iommu_group)) {
+			vfio_iommu_detach_group(domain, group);
+			if (!vfio_iommu_attach_group(d, group)) {
 				list_add(&group->next, &d->group_list);
 				iommu_domain_free(domain->domain);
 				kfree(domain);
@@ -1414,7 +1484,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 				return 0;
 			}
 
-			ret = iommu_attach_group(domain->domain, iommu_group);
+			ret = vfio_iommu_attach_group(domain, group);
 			if (ret)
 				goto out_domain;
 		}
@@ -1440,7 +1510,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	return 0;
 
 out_detach:
-	iommu_detach_group(domain->domain, iommu_group);
+	vfio_iommu_detach_group(domain, group);
 out_domain:
 	iommu_domain_free(domain->domain);
 out_free:
@@ -1531,7 +1601,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
 		if (!group)
 			continue;
 
-		iommu_detach_group(domain->domain, iommu_group);
+		vfio_iommu_detach_group(domain, group);
 		list_del(&group->next);
 		kfree(group);
 		/*
@@ -1596,7 +1666,7 @@ static void vfio_release_domain(struct vfio_domain *domain, bool external)
 	list_for_each_entry_safe(group, group_tmp,
 				 &domain->group_list, next) {
 		if (!external)
-			iommu_detach_group(domain->domain, group->iommu_group);
+			vfio_iommu_detach_group(domain, group);
 		list_del(&group->next);
 		kfree(group);
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 9/9] vfio/type1: Handle different mdev isolation type
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
                   ` (7 preceding siblings ...)
  2019-02-13  4:03 ` [PATCH v6 8/9] vfio/type1: Add domain at(de)taching group helpers Lu Baolu
@ 2019-02-13  4:03 ` Lu Baolu
  2019-02-14 20:14 ` [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Alex Williamson
  9 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-13  4:03 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel, Lu Baolu, Jacob Pan

This adds the support to determine the isolation type
of a mediated device group by checking whether it has
an iommu device. If an iommu device exists, an iommu
domain will be allocated and then attached to the iommu
device. Otherwise, keep the same behavior as it is.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Sanjay Kumar <sanjay.k.kumar@intel.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/vfio/vfio_iommu_type1.c | 48 ++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index ccc4165474aa..f1392c582a3c 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1368,13 +1368,40 @@ static void vfio_iommu_detach_group(struct vfio_domain *domain,
 		iommu_detach_group(domain->domain, group->iommu_group);
 }
 
+static bool vfio_bus_is_mdev(struct bus_type *bus)
+{
+	struct bus_type *mdev_bus;
+	bool ret = false;
+
+	mdev_bus = symbol_get(mdev_bus_type);
+	if (mdev_bus) {
+		ret = (bus == mdev_bus);
+		symbol_put(mdev_bus_type);
+	}
+
+	return ret;
+}
+
+static int vfio_mdev_iommu_device(struct device *dev, void *data)
+{
+	struct device **old = data, *new;
+
+	new = vfio_mdev_get_iommu_device(dev);
+	if (!new || (*old && *old != new))
+		return -EINVAL;
+
+	*old = new;
+
+	return 0;
+}
+
 static int vfio_iommu_type1_attach_group(void *iommu_data,
 					 struct iommu_group *iommu_group)
 {
 	struct vfio_iommu *iommu = iommu_data;
 	struct vfio_group *group;
 	struct vfio_domain *domain, *d;
-	struct bus_type *bus = NULL, *mdev_bus;
+	struct bus_type *bus = NULL;
 	int ret;
 	bool resv_msi, msi_remap;
 	phys_addr_t resv_msi_base;
@@ -1409,23 +1436,30 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	if (ret)
 		goto out_free;
 
-	mdev_bus = symbol_get(mdev_bus_type);
+	if (vfio_bus_is_mdev(bus)) {
+		struct device *iommu_device = NULL;
 
-	if (mdev_bus) {
-		if ((bus == mdev_bus) && !iommu_present(bus)) {
-			symbol_put(mdev_bus_type);
+		group->mdev_group = true;
+
+		/* Determine the isolation type */
+		ret = iommu_group_for_each_dev(iommu_group, &iommu_device,
+					       vfio_mdev_iommu_device);
+		if (ret || !iommu_device) {
 			if (!iommu->external_domain) {
 				INIT_LIST_HEAD(&domain->group_list);
 				iommu->external_domain = domain;
-			} else
+			} else {
 				kfree(domain);
+			}
 
 			list_add(&group->next,
 				 &iommu->external_domain->group_list);
 			mutex_unlock(&iommu->lock);
+
 			return 0;
 		}
-		symbol_put(mdev_bus_type);
+
+		bus = iommu_device->bus;
 	}
 
 	domain->domain = iommu_domain_alloc(bus);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 1/9] iommu: Add APIs for multiple domains per device
  2019-02-13  4:02 ` [PATCH v6 1/9] iommu: Add APIs for multiple domains per device Lu Baolu
@ 2019-02-13 11:55   ` Jean-Philippe Brucker
  2019-02-14  1:34     ` Lu Baolu
  0 siblings, 1 reply; 19+ messages in thread
From: Jean-Philippe Brucker @ 2019-02-13 11:55 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, David Woodhouse, Alex Williamson, Kirti Wankhede
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, peterx, tiwei.bie, xin.zeng, iommu, kvm, linux-kernel,
	Jacob Pan

Hi,

I have a few boring nits and one question below

On 13/02/2019 04:02, Lu Baolu wrote:
> Sharing a physical PCI device in a finer-granularity way
> is becoming a consensus in the industry. IOMMU vendors
> are also engaging efforts to support such sharing as well
> as possible. Among the efforts, the capability of support
> finer-granularity DMA isolation is a common requirement
> due to the security consideration. With finer-granularity
> DMA isolation, all DMA requests out of or to a subset of
> a physical PCI device can be protected by the IOMMU.

That last sentence seems strange, how about "With finer-granularity DMA
isolation, subsets of a PCI function can be isolated from each others by
the IOMMU."

> As a
> result, there is a request in software to attach multiple
> domains to a physical PCI device. One example of such use
> model is the Intel Scalable IOV [1] [2]. The Intel vt-d
> 3.0 spec [3] introduces the scalable mode which enables
> PASID granularity DMA isolation.
> 
> This adds the APIs to support multiple domains per device.
> In order to ease the discussions, we call it 'a domain in
> auxiliary mode' or simply 'auxiliary domain' when multiple
> domains are attached to a physical device.
> 
> The APIs include:
> 
> * iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
>   - Check whether both IOMMU and device support IOMMU aux
>     domain feature. Below aux-domain specific interfaces
>     are available only after this returns true.

s/after/if/ since calling has_feature() shouldn't be a prerequisite to
using the aux-domain interface (unlike calling enable_feature()).

> 
> * iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_AUX)
>   - Enable/disable device specific aux-domain feature.
> 
> * iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
>   - Check whether the aux domain specific feature enabled or
>     not.

"is enabled"

> 
> * iommu_aux_attach_device(domain, dev)
>   - Attaches @domain to @dev in the auxiliary mode. Multiple
>     domains could be attached to a single device in the
>     auxiliary mode with each domain representing an isolated
>     address space for an assignable subset of the device.
> 
> * iommu_aux_detach_device(domain, dev)
>   - Detach @domain which has been attached to @dev in the
>     auxiliary mode.
> 
> * iommu_aux_get_pasid(domain, dev)
>   - Return ID used for finer-granularity DMA translation.
>     For the Intel Scalable IOV usage model, this will be
>     a PASID. The device which supports Scalable IOV needs
>     to write this ID to the device register so that DMA
>     requests could be tagged with a right PASID prefix.
> 
> This has been updated with the latest proposal from Joerg
> posted here [5].
> 
> Many people involved in discussions of this design.
> 
> Kevin Tian <kevin.tian@intel.com>
> Liu Yi L <yi.l.liu@intel.com>
> Ashok Raj <ashok.raj@intel.com>
> Sanjay Kumar <sanjay.k.kumar@intel.com>
> Jacob Pan <jacob.jun.pan@linux.intel.com>
> Alex Williamson <alex.williamson@redhat.com>
> Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Joerg Roedel <joro@8bytes.org>
> 
> and some discussions can be found here [4] [5].
> 
> [1]
> https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
> [2] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
> [3]
> https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
> [4] https://lkml.org/lkml/2018/7/26/4
> [5] https://www.spinics.net/lists/iommu/msg31874.html
> 
> Cc: Ashok Raj <ashok.raj@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Liu Yi L <yi.l.liu@intel.com>
> Suggested-by: Kevin Tian <kevin.tian@intel.com>
> Suggested-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Suggested-by: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/iommu.c | 91 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iommu.h | 70 +++++++++++++++++++++++++++++++++
>  2 files changed, 161 insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 3ed4db334341..d0b323e8357f 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2033,3 +2033,94 @@ int iommu_fwspec_add_ids(struct device *dev, u32
> *ids, int num_ids)
>          return 0;
>  }
>  EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
> +
> +/*
> + * Per device IOMMU features.
> + */
> +bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features
> feat)
> +{
> +       const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +       if (ops && ops->dev_has_feat)
> +               return ops->dev_has_feat(dev, feat);
> +
> +       return false;
> +}
> +EXPORT_SYMBOL_GPL(iommu_dev_has_feature);
> +
> +int iommu_dev_enable_feature(struct device *dev, enum
> iommu_dev_features feat)
> +{
> +       const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +       if (ops && ops->dev_enable_feat)
> +               return ops->dev_enable_feat(dev, feat);
> +
> +       return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(iommu_dev_enable_feature);
> +
> +int iommu_dev_disable_feature(struct device *dev, enum
> iommu_dev_features feat)
> +{
> +       const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +       if (ops && ops->dev_disable_feat)
> +               return ops->dev_disable_feat(dev, feat);
> +
> +       return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(iommu_dev_disable_feature);
> +
> +bool iommu_dev_feature_enabled(struct device *dev, enum
> iommu_dev_features feat)
> +{
> +       const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +       if (ops && ops->dev_feat_enabled)
> +               return ops->dev_feat_enabled(dev, feat);
> +
> +       return false;
> +}
> +EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
> +
> +/*
> + * Aux-domain specific attach/detach.
> + *
> + * Only works if iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX) returns
> true.

Rather "Only works if iommu_dev_feature_enabled(dev, AUX) returns true"?

> + * Also, as long as domains are attached to a device through this
> interface,
> + * any tries to call iommu_attach_device() should fail
> (iommu_detach_device()
> + * can't fail, so we fail on the tryint to re-attach). This should make

"when trying to re-attach"

> us safe
> + * against a device being attached to a guest as a whole while there
> are still
> + * pasid users on it (aux and sva).

A related question: is calling iommu_dev_disable_feat(dev, AUX) enough
to detach all auxiliary domains? I guess device drivers will always need
to tidy contexts up one by one, so maybe iommu_dev_disable_feat(dev,
AUX) can return -EBUSY in this case? I think we need to specify this in
a comment so device drivers know how to clean up.

Thanks,
Jean

> + */
> +int iommu_aux_attach_device(struct iommu_domain *domain, struct device
> *dev)
> +{
> +       int ret = -ENODEV;
> +
> +       if (domain->ops->aux_attach_dev)
> +               ret = domain->ops->aux_attach_dev(domain, dev);
> +
> +       if (!ret)
> +               trace_attach_device_to_domain(dev);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
> +
> +void iommu_aux_detach_device(struct iommu_domain *domain, struct device
> *dev)
> +{
> +       if (domain->ops->aux_detach_dev) {
> +               domain->ops->aux_detach_dev(domain, dev);
> +               trace_detach_device_from_domain(dev);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
> +
> +int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
> +{
> +       int ret = -ENODEV;
> +
> +       if (domain->ops->aux_get_pasid)
> +               ret = domain->ops->aux_get_pasid(domain, dev);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e90da6b6f3d1..831eb29b35c5 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -156,6 +156,11 @@ struct iommu_resv_region {
>          enum iommu_resv_type    type;
>  };
>  
> +/* Per device IOMMU features */
> +enum iommu_dev_features {
> +       IOMMU_DEV_FEAT_AUX,     /* Aux-domain feature */
> +};
> +
>  #ifdef CONFIG_IOMMU_API
>  
>  /**
> @@ -183,6 +188,11 @@ struct iommu_resv_region {
>   * @domain_window_enable: Configure and enable a particular window for
> a domain
>   * @domain_window_disable: Disable a particular window for a domain
>   * @of_xlate: add OF master IDs to iommu grouping
> + * @dev_has/enable/disable_feat: per device entries to check/enable/disable
> + *                               iommu specific features.
> + * @dev_feat_enabled: check enabled feature
> + * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
> + * @aux_get_pasid: get the pasid given an aux-domain
>   * @pgsize_bitmap: bitmap of all possible supported page sizes
>   */
>  struct iommu_ops {
> @@ -226,6 +236,17 @@ struct iommu_ops {
>          int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
>          bool (*is_attach_deferred)(struct iommu_domain *domain, struct
> device *dev);
>  
> +       /* Per device IOMMU features */
> +       bool (*dev_has_feat)(struct device *dev, enum iommu_dev_features f);
> +       bool (*dev_feat_enabled)(struct device *dev, enum
> iommu_dev_features f);
> +       int (*dev_enable_feat)(struct device *dev, enum
> iommu_dev_features f);
> +       int (*dev_disable_feat)(struct device *dev, enum
> iommu_dev_features f);
> +
> +       /* Aux-domain specific attach/detach entries */
> +       int (*aux_attach_dev)(struct iommu_domain *domain, struct device
> *dev);
> +       void (*aux_detach_dev)(struct iommu_domain *domain, struct
> device *dev);
> +       int (*aux_get_pasid)(struct iommu_domain *domain, struct device
> *dev);
> +
>          unsigned long pgsize_bitmap;
>  };
>  
> @@ -412,6 +433,14 @@ static inline void dev_iommu_fwspec_set(struct
> device *dev,
>  int iommu_probe_device(struct device *dev);
>  void iommu_release_device(struct device *dev);
>  
> +bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features f);
> +int iommu_dev_enable_feature(struct device *dev, enum
> iommu_dev_features f);
> +int iommu_dev_disable_feature(struct device *dev, enum
> iommu_dev_features f);
> +bool iommu_dev_feature_enabled(struct device *dev, enum
> iommu_dev_features f);
> +int iommu_aux_attach_device(struct iommu_domain *domain, struct device
> *dev);
> +void iommu_aux_detach_device(struct iommu_domain *domain, struct device
> *dev);
> +int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev);
> +
>  #else /* CONFIG_IOMMU_API */
>  
>  struct iommu_ops {};
> @@ -696,6 +725,47 @@ const struct iommu_ops
> *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
>          return NULL;
>  }
>  
> +static inline bool
> +iommu_dev_has_feature(struct device *dev, enum iommu_dev_features feat)
> +{
> +       return false;
> +}
> +
> +static inline bool
> +iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features feat)
> +{
> +       return false;
> +}
> +
> +static inline int
> +iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
> +{
> +       return -ENODEV;
> +}
> +
> +static inline int
> +iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat)
> +{
> +       return -ENODEV;
> +}
> +
> +static inline int
> +iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
> +{
> +       return -ENODEV;
> +}
> +
> +static inline void
> +iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
> +{
> +}
> +
> +static inline int
> +iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
> +{
> +       return -ENODEV;
> +}
> +
>  #endif /* CONFIG_IOMMU_API */
>  
>  #ifdef CONFIG_IOMMU_DEBUGFS
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 1/9] iommu: Add APIs for multiple domains per device
  2019-02-13 11:55   ` Jean-Philippe Brucker
@ 2019-02-14  1:34     ` Lu Baolu
  0 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-14  1:34 UTC (permalink / raw)
  To: Jean-Philippe Brucker, Joerg Roedel, David Woodhouse,
	Alex Williamson, Kirti Wankhede
  Cc: baolu.lu, ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	yi.l.liu, yi.y.sun, peterx, tiwei.bie, xin.zeng, iommu, kvm,
	linux-kernel, Jacob Pan

Hi Jean,

On 2/13/19 7:55 PM, Jean-Philippe Brucker wrote:
> Hi,
> 
> I have a few boring nits and one question below

Thanks a lot for reviewing my patch.

> 
> On 13/02/2019 04:02, Lu Baolu wrote:
>> Sharing a physical PCI device in a finer-granularity way
>> is becoming a consensus in the industry. IOMMU vendors
>> are also engaging efforts to support such sharing as well
>> as possible. Among the efforts, the capability of support
>> finer-granularity DMA isolation is a common requirement
>> due to the security consideration. With finer-granularity
>> DMA isolation, all DMA requests out of or to a subset of
>> a physical PCI device can be protected by the IOMMU.
> 
> That last sentence seems strange, how about "With finer-granularity DMA
> isolation, subsets of a PCI function can be isolated from each others by
> the IOMMU."

Yours looks better. Thanks!

> 
>> As a
>> result, there is a request in software to attach multiple
>> domains to a physical PCI device. One example of such use
>> model is the Intel Scalable IOV [1] [2]. The Intel vt-d
>> 3.0 spec [3] introduces the scalable mode which enables
>> PASID granularity DMA isolation.
>>
>> This adds the APIs to support multiple domains per device.
>> In order to ease the discussions, we call it 'a domain in
>> auxiliary mode' or simply 'auxiliary domain' when multiple
>> domains are attached to a physical device.
>>
>> The APIs include:
>>
>> * iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
>>    - Check whether both IOMMU and device support IOMMU aux
>>      domain feature. Below aux-domain specific interfaces
>>      are available only after this returns true.
> 
> s/after/if/ since calling has_feature() shouldn't be a prerequisite to
> using the aux-domain interface (unlike calling enable_feature()).

After reconsideration, I think my comments about this API isn't
correct. It should be like,

* iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
   - Detect both IOMMU and PCI endpoint devices supporting the feature
     (aux-domain here) without host driver dependency.

* iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
   - Check the enabling status of the feature (aux-domain here). The
     aux-domain interfaces are available only if this returns true.

> 
>>
>> * iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_AUX)
>>    - Enable/disable device specific aux-domain feature.
>>
>> * iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
>>    - Check whether the aux domain specific feature enabled or
>>      not.
> 
> "is enabled"

Sure.

> 
>>
>> * iommu_aux_attach_device(domain, dev)
>>    - Attaches @domain to @dev in the auxiliary mode. Multiple
>>      domains could be attached to a single device in the
>>      auxiliary mode with each domain representing an isolated
>>      address space for an assignable subset of the device.
>>
>> * iommu_aux_detach_device(domain, dev)
>>    - Detach @domain which has been attached to @dev in the
>>      auxiliary mode.
>>
>> * iommu_aux_get_pasid(domain, dev)
>>    - Return ID used for finer-granularity DMA translation.
>>      For the Intel Scalable IOV usage model, this will be
>>      a PASID. The device which supports Scalable IOV needs
>>      to write this ID to the device register so that DMA
>>      requests could be tagged with a right PASID prefix.
>>
>> This has been updated with the latest proposal from Joerg
>> posted here [5].
>>
>> Many people involved in discussions of this design.
>>
>> Kevin Tian <kevin.tian@intel.com>
>> Liu Yi L <yi.l.liu@intel.com>
>> Ashok Raj <ashok.raj@intel.com>
>> Sanjay Kumar <sanjay.k.kumar@intel.com>
>> Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Alex Williamson <alex.williamson@redhat.com>
>> Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> Joerg Roedel <joro@8bytes.org>
>>
>> and some discussions can be found here [4] [5].
>>
>> [1]
>> https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
>> [2] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
>> [3]
>> https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
>> [4] https://lkml.org/lkml/2018/7/26/4
>> [5] https://www.spinics.net/lists/iommu/msg31874.html
>>
>> Cc: Ashok Raj <ashok.raj@intel.com>
>> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>> Cc: Liu Yi L <yi.l.liu@intel.com>
>> Suggested-by: Kevin Tian <kevin.tian@intel.com>
>> Suggested-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> Suggested-by: Joerg Roedel <jroedel@suse.de>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   drivers/iommu/iommu.c | 91 +++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/iommu.h | 70 +++++++++++++++++++++++++++++++++
>>   2 files changed, 161 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 3ed4db334341..d0b323e8357f 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -2033,3 +2033,94 @@ int iommu_fwspec_add_ids(struct device *dev, u32
>> *ids, int num_ids)
>>           return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
>> +
>> +/*
>> + * Per device IOMMU features.
>> + */
>> +bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features
>> feat)
>> +{
>> +       const struct iommu_ops *ops = dev->bus->iommu_ops;
>> +
>> +       if (ops && ops->dev_has_feat)
>> +               return ops->dev_has_feat(dev, feat);
>> +
>> +       return false;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_dev_has_feature);
>> +
>> +int iommu_dev_enable_feature(struct device *dev, enum
>> iommu_dev_features feat)
>> +{
>> +       const struct iommu_ops *ops = dev->bus->iommu_ops;
>> +
>> +       if (ops && ops->dev_enable_feat)
>> +               return ops->dev_enable_feat(dev, feat);
>> +
>> +       return -ENODEV;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_dev_enable_feature);
>> +
>> +int iommu_dev_disable_feature(struct device *dev, enum
>> iommu_dev_features feat)
>> +{
>> +       const struct iommu_ops *ops = dev->bus->iommu_ops;
>> +
>> +       if (ops && ops->dev_disable_feat)
>> +               return ops->dev_disable_feat(dev, feat);
>> +
>> +       return -ENODEV;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_dev_disable_feature);
>> +
>> +bool iommu_dev_feature_enabled(struct device *dev, enum
>> iommu_dev_features feat)
>> +{
>> +       const struct iommu_ops *ops = dev->bus->iommu_ops;
>> +
>> +       if (ops && ops->dev_feat_enabled)
>> +               return ops->dev_feat_enabled(dev, feat);
>> +
>> +       return false;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
>> +
>> +/*
>> + * Aux-domain specific attach/detach.
>> + *
>> + * Only works if iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX) returns
>> true.
> 
> Rather "Only works if iommu_dev_feature_enabled(dev, AUX) returns true"?

Yes. I will change this and above.

> 
>> + * Also, as long as domains are attached to a device through this
>> interface,
>> + * any tries to call iommu_attach_device() should fail
>> (iommu_detach_device()
>> + * can't fail, so we fail on the tryint to re-attach). This should make
> 
> "when trying to re-attach"

Sure.

> 
>> us safe
>> + * against a device being attached to a guest as a whole while there
>> are still
>> + * pasid users on it (aux and sva).
> 
> A related question: is calling iommu_dev_disable_feat(dev, AUX) enough
> to detach all auxiliary domains? I guess device drivers will always need
> to tidy contexts up one by one, so maybe iommu_dev_disable_feat(dev,
> AUX) can return -EBUSY in this case? I think we need to specify this in
> a comment so device drivers know how to clean up.

Yes, -EBUSY sounds better.

iommu_dev_disable_feat() will not detach all aux-domains, it requires
the device drivers to do so before calling it. So if it sees anything
not cleaned up, it should return -EBUSY. And we should put it as a
comment for this API so that the device drivers know it.

> 
> Thanks,
> Jean

Best regards,
Lu Baolu

> 
>> + */
>> +int iommu_aux_attach_device(struct iommu_domain *domain, struct device
>> *dev)
>> +{
>> +       int ret = -ENODEV;
>> +
>> +       if (domain->ops->aux_attach_dev)
>> +               ret = domain->ops->aux_attach_dev(domain, dev);
>> +
>> +       if (!ret)
>> +               trace_attach_device_to_domain(dev);
>> +
>> +       return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
>> +
>> +void iommu_aux_detach_device(struct iommu_domain *domain, struct device
>> *dev)
>> +{
>> +       if (domain->ops->aux_detach_dev) {
>> +               domain->ops->aux_detach_dev(domain, dev);
>> +               trace_detach_device_from_domain(dev);
>> +       }
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
>> +
>> +int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
>> +{
>> +       int ret = -ENODEV;
>> +
>> +       if (domain->ops->aux_get_pasid)
>> +               ret = domain->ops->aux_get_pasid(domain, dev);
>> +
>> +       return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index e90da6b6f3d1..831eb29b35c5 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -156,6 +156,11 @@ struct iommu_resv_region {
>>           enum iommu_resv_type    type;
>>   };
>>   
>> +/* Per device IOMMU features */
>> +enum iommu_dev_features {
>> +       IOMMU_DEV_FEAT_AUX,     /* Aux-domain feature */
>> +};
>> +
>>   #ifdef CONFIG_IOMMU_API
>>   
>>   /**
>> @@ -183,6 +188,11 @@ struct iommu_resv_region {
>>    * @domain_window_enable: Configure and enable a particular window for
>> a domain
>>    * @domain_window_disable: Disable a particular window for a domain
>>    * @of_xlate: add OF master IDs to iommu grouping
>> + * @dev_has/enable/disable_feat: per device entries to check/enable/disable
>> + *                               iommu specific features.
>> + * @dev_feat_enabled: check enabled feature
>> + * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
>> + * @aux_get_pasid: get the pasid given an aux-domain
>>    * @pgsize_bitmap: bitmap of all possible supported page sizes
>>    */
>>   struct iommu_ops {
>> @@ -226,6 +236,17 @@ struct iommu_ops {
>>           int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
>>           bool (*is_attach_deferred)(struct iommu_domain *domain, struct
>> device *dev);
>>   
>> +       /* Per device IOMMU features */
>> +       bool (*dev_has_feat)(struct device *dev, enum iommu_dev_features f);
>> +       bool (*dev_feat_enabled)(struct device *dev, enum
>> iommu_dev_features f);
>> +       int (*dev_enable_feat)(struct device *dev, enum
>> iommu_dev_features f);
>> +       int (*dev_disable_feat)(struct device *dev, enum
>> iommu_dev_features f);
>> +
>> +       /* Aux-domain specific attach/detach entries */
>> +       int (*aux_attach_dev)(struct iommu_domain *domain, struct device
>> *dev);
>> +       void (*aux_detach_dev)(struct iommu_domain *domain, struct
>> device *dev);
>> +       int (*aux_get_pasid)(struct iommu_domain *domain, struct device
>> *dev);
>> +
>>           unsigned long pgsize_bitmap;
>>   };
>>   
>> @@ -412,6 +433,14 @@ static inline void dev_iommu_fwspec_set(struct
>> device *dev,
>>   int iommu_probe_device(struct device *dev);
>>   void iommu_release_device(struct device *dev);
>>   
>> +bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features f);
>> +int iommu_dev_enable_feature(struct device *dev, enum
>> iommu_dev_features f);
>> +int iommu_dev_disable_feature(struct device *dev, enum
>> iommu_dev_features f);
>> +bool iommu_dev_feature_enabled(struct device *dev, enum
>> iommu_dev_features f);
>> +int iommu_aux_attach_device(struct iommu_domain *domain, struct device
>> *dev);
>> +void iommu_aux_detach_device(struct iommu_domain *domain, struct device
>> *dev);
>> +int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev);
>> +
>>   #else /* CONFIG_IOMMU_API */
>>   
>>   struct iommu_ops {};
>> @@ -696,6 +725,47 @@ const struct iommu_ops
>> *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
>>           return NULL;
>>   }
>>   
>> +static inline bool
>> +iommu_dev_has_feature(struct device *dev, enum iommu_dev_features feat)
>> +{
>> +       return false;
>> +}
>> +
>> +static inline bool
>> +iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features feat)
>> +{
>> +       return false;
>> +}
>> +
>> +static inline int
>> +iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
>> +{
>> +       return -ENODEV;
>> +}
>> +
>> +static inline int
>> +iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat)
>> +{
>> +       return -ENODEV;
>> +}
>> +
>> +static inline int
>> +iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
>> +{
>> +       return -ENODEV;
>> +}
>> +
>> +static inline void
>> +iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
>> +{
>> +}
>> +
>> +static inline int
>> +iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
>> +{
>> +       return -ENODEV;
>> +}
>> +
>>   #endif /* CONFIG_IOMMU_API */
>>   
>>   #ifdef CONFIG_IOMMU_DEBUGFS
>> -- 
>> 2.17.1
>>
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
  2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
                   ` (8 preceding siblings ...)
  2019-02-13  4:03 ` [PATCH v6 9/9] vfio/type1: Handle different mdev isolation type Lu Baolu
@ 2019-02-14 20:14 ` Alex Williamson
  2019-02-15 18:46     ` Jean-Philippe Brucker
                     ` (3 more replies)
  9 siblings, 4 replies; 19+ messages in thread
From: Alex Williamson @ 2019-02-14 20:14 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Joerg Roedel, David Woodhouse, Kirti Wankhede, ashok.raj,
	sanjay.k.kumar, jacob.jun.pan, kevin.tian, Jean-Philippe Brucker,
	yi.l.liu, yi.y.sun, peterx, tiwei.bie, xin.zeng, iommu, kvm,
	linux-kernel

On Wed, 13 Feb 2019 12:02:52 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

> Hi,
> 
> The Mediate Device is a framework for fine-grained physical device
> sharing across the isolated domains. Currently the mdev framework
> is designed to be independent of the platform IOMMU support. As the
> result, the DMA isolation relies on the mdev parent device in a
> vendor specific way.
> 
> There are several cases where a mediated device could be protected
> and isolated by the platform IOMMU. For example, Intel vt-d rev3.0
> [1] introduces a new translation mode called 'scalable mode', which
> enables PASID-granular translations. The vt-d scalable mode is the
> key ingredient for Scalable I/O Virtualization [2] [3] which allows
> sharing a device in minimal possible granularity (ADI - Assignable
> Device Interface).
> 
> A mediated device backed by an ADI could be protected and isolated
> by the IOMMU since 1) the parent device supports tagging an unique
> PASID to all DMA traffic out of the mediated device; and 2) the DMA
> translation unit (IOMMU) supports the PASID granular translation.
> We can apply IOMMU protection and isolation to this kind of devices
> just as what we are doing with an assignable PCI device.
> 
> In order to distinguish the IOMMU-capable mediated devices from those
> which still need to rely on parent devices, this patch set adds one
> new member in struct mdev_device.
> 
> * iommu_device
>   - This, if set, indicates that the mediated device could
>     be fully isolated and protected by IOMMU via attaching
>     an iommu domain to this device. If empty, it indicates
>     using vendor defined isolation.
> 
> Below helpers are added to set and get above iommu device in mdev core
> implementation.
> 
> * mdev_set/get_iommu_device(dev, iommu_device)
>   - Set or get the iommu device which represents this mdev
>     in IOMMU's device scope. Drivers don't need to set the
>     iommu device if it uses vendor defined isolation.
> 
> The mdev parent device driver could opt-in that the mdev could be
> fully isolated and protected by the IOMMU when the mdev is being
> created by invoking mdev_set_iommu_device() in its @create().
> 
> In the vfio_iommu_type1_attach_group(), a domain allocated through
> iommu_domain_alloc() will be attached to the mdev iommu device if
> an iommu device has been set. Otherwise, the dummy external domain
> will be used and all the DMA isolation and protection are routed to
> parent driver as the result.
> 
> On IOMMU side, a basic requirement is allowing to attach multiple
> domains to a PCI device if the device advertises the capability
> and the IOMMU hardware supports finer granularity translations than
> the normal PCI Source ID based translation.
> 
> As the result, a PCI device could work in two modes: normal mode
> and auxiliary mode. In the normal mode, a pci device could be
> isolated in the Source ID granularity; the pci device itself could
> be assigned to a user application by attaching a single domain
> to it. In the auxiliary mode, a pci device could be isolated in
> finer granularity, hence subsets of the device could be assigned
> to different user level application by attaching a different domain
> to each subset.
> 
> Below APIs are introduced in iommu generic layer for aux-domain
> purpose:
> 
> * iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
>   - Check whether both IOMMU and device support IOMMU aux
>     domain feature. Below aux-domain specific interfaces
>     are available only after this returns true.
> 
> * iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_AUX)
>   - Enable/disable device specific aux-domain feature.
> 
> * iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
>   - Check whether the aux domain specific feature enabled or
>     not.
> 
> * iommu_aux_attach_device(domain, dev)
>   - Attaches @domain to @dev in the auxiliary mode. Multiple
>     domains could be attached to a single device in the
>     auxiliary mode with each domain representing an isolated
>     address space for an assignable subset of the device.
> 
> * iommu_aux_detach_device(domain, dev)
>   - Detach @domain which has been attached to @dev in the
>     auxiliary mode.
> 
> * iommu_aux_get_pasid(domain, dev)
>   - Return ID used for finer-granularity DMA translation.
>     For the Intel Scalable IOV usage model, this will be
>     a PASID. The device which supports Scalable IOV needs
>     to write this ID to the device register so that DMA
>     requests could be tagged with a right PASID prefix.
> 
> In order for the ease of discussion, sometimes we call "a domain in
> auxiliary mode' or simply 'an auxiliary domain' when a domain is
> attached to a device for finer granularity translations. But we need
> to keep in mind that this doesn't mean there is a differnt domain
> type. A same domain could be bound to a device for Source ID based
> translation, and bound to another device for finer granularity
> translation at the same time.
> 
> This patch series extends both IOMMU and vfio components to support
> mdev device passing through when it could be isolated and protected
> by the IOMMU units. The first part of this series (PATCH 1/09~6/09)
> adds the interfaces and implementation of the multiple domains per
> device. The second part (PATCH 7/09~9/09) adds the iommu device
> attribute to each mdev, determines isolation type according to the
> existence of an iommu device when attaching group in vfio type1 iommu
> module, and attaches the domain to iommu aware mediated devices.
> 
> References:
> [1] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
> [2] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
> [3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
> 
> Best regards,
> Lu Baolu
> 
> Change log:
>   v5->v6:

This looks pretty reasonable with Jean-Philippe's nit fixups.  Where do
we go from here?  I think we need an ack from Kirti since they have an
interest here.  Presumably this looks ok to the ARM folks.  Do we have
any consumers of this code yet?  Theoretically I think a vfio-pci-like
meta driver could be written as an mdev vendor driver with this support
(restricted to type1 iommu use cases).  Thanks,

Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
@ 2019-02-15 18:46     ` Jean-Philippe Brucker
  0 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2019-02-15 18:46 UTC (permalink / raw)
  To: Alex Williamson, Lu Baolu
  Cc: Joerg Roedel, David Woodhouse, Kirti Wankhede, ashok.raj,
	sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu, yi.y.sun,
	peterx, tiwei.bie, xin.zeng, iommu, kvm, linux-kernel

On 14/02/2019 20:14, Alex Williamson wrote:
>> This patch series extends both IOMMU and vfio components to support
>> mdev device passing through when it could be isolated and protected
>> by the IOMMU units. The first part of this series (PATCH 1/09~6/09)
>> adds the interfaces and implementation of the multiple domains per
>> device. The second part (PATCH 7/09~9/09) adds the iommu device
>> attribute to each mdev, determines isolation type according to the
>> existence of an iommu device when attaching group in vfio type1 iommu
>> module, and attaches the domain to iommu aware mediated devices.
>> 
>> References:
>> [1] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
>> [2] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
>> [3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
>> 
>> Best regards,
>> Lu Baolu
>> 
>> Change log:
>>   v5->v6:
> 
> This looks pretty reasonable with Jean-Philippe's nit fixups.  Where do
> we go from here?  I think we need an ack from Kirti since they have an
> interest here.  Presumably this looks ok to the ARM folks.

Looks great from my point of view. I focused on patch 1 since I'm
planning to reuse iommu_dev_features for SVA. I don't have time to test
auxd and mdev on SMMUv3 at the moment but I had a better look and, if it
helps, for patches 1 and 7-9:

Reviewed-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

That said, are you planning to add back the mdev_get_iommu_domain()
function, in a separate patch? Because I think the parent driver still
needs a way to retrieve the PASID for an mdev?

Thanks,
Jean

> Do we have
> any consumers of this code yet?  Theoretically I think a vfio-pci-like
> meta driver could be written as an mdev vendor driver with this support
> (restricted to type1 iommu use cases).  Thanks,
> 
> Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
@ 2019-02-15 18:46     ` Jean-Philippe Brucker
  0 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2019-02-15 18:46 UTC (permalink / raw)
  To: Alex Williamson, Lu Baolu
  Cc: kevin.tian-ral2JQCrhuEAvxtiuMwx3w,
	ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	tiwei.bie-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Kirti Wankhede,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w, kvm-u79uwXL29TY76Z2rM5mHXA,
	David Woodhouse

On 14/02/2019 20:14, Alex Williamson wrote:
>> This patch series extends both IOMMU and vfio components to support
>> mdev device passing through when it could be isolated and protected
>> by the IOMMU units. The first part of this series (PATCH 1/09~6/09)
>> adds the interfaces and implementation of the multiple domains per
>> device. The second part (PATCH 7/09~9/09) adds the iommu device
>> attribute to each mdev, determines isolation type according to the
>> existence of an iommu device when attaching group in vfio type1 iommu
>> module, and attaches the domain to iommu aware mediated devices.
>> 
>> References:
>> [1] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
>> [2] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
>> [3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
>> 
>> Best regards,
>> Lu Baolu
>> 
>> Change log:
>>   v5->v6:
> 
> This looks pretty reasonable with Jean-Philippe's nit fixups.  Where do
> we go from here?  I think we need an ack from Kirti since they have an
> interest here.  Presumably this looks ok to the ARM folks.

Looks great from my point of view. I focused on patch 1 since I'm
planning to reuse iommu_dev_features for SVA. I don't have time to test
auxd and mdev on SMMUv3 at the moment but I had a better look and, if it
helps, for patches 1 and 7-9:

Reviewed-by: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>

That said, are you planning to add back the mdev_get_iommu_domain()
function, in a separate patch? Because I think the parent driver still
needs a way to retrieve the PASID for an mdev?

Thanks,
Jean

> Do we have
> any consumers of this code yet?  Theoretically I think a vfio-pci-like
> meta driver could be written as an mdev vendor driver with this support
> (restricted to type1 iommu use cases).  Thanks,
> 
> Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
  2019-02-15 18:46     ` Jean-Philippe Brucker
  (?)
@ 2019-02-19  2:46     ` Lu Baolu
  -1 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-19  2:46 UTC (permalink / raw)
  To: Jean-Philippe Brucker, Alex Williamson
  Cc: baolu.lu, Joerg Roedel, David Woodhouse, Kirti Wankhede,
	ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, peterx, tiwei.bie, xin.zeng, iommu, kvm, linux-kernel

Hi Jean,

On 2/16/19 2:46 AM, Jean-Philippe Brucker wrote:
> On 14/02/2019 20:14, Alex Williamson wrote:
>>> This patch series extends both IOMMU and vfio components to support
>>> mdev device passing through when it could be isolated and protected
>>> by the IOMMU units. The first part of this series (PATCH 1/09~6/09)
>>> adds the interfaces and implementation of the multiple domains per
>>> device. The second part (PATCH 7/09~9/09) adds the iommu device
>>> attribute to each mdev, determines isolation type according to the
>>> existence of an iommu device when attaching group in vfio type1 iommu
>>> module, and attaches the domain to iommu aware mediated devices.
>>>
>>> References:
>>> [1] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
>>> [2] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
>>> [3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
>>>
>>> Best regards,
>>> Lu Baolu
>>>
>>> Change log:
>>>     v5->v6:
>>
>> This looks pretty reasonable with Jean-Philippe's nit fixups.  Where do
>> we go from here?  I think we need an ack from Kirti since they have an
>> interest here.  Presumably this looks ok to the ARM folks.
> 
> Looks great from my point of view. I focused on patch 1 since I'm
> planning to reuse iommu_dev_features for SVA. I don't have time to test
> auxd and mdev on SMMUv3 at the moment but I had a better look and, if it
> helps, for patches 1 and 7-9:
> 
> Reviewed-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

Thank you! I will add this in the next version.

> 
> That said, are you planning to add back the mdev_get_iommu_domain()
> function, in a separate patch? Because I think the parent driver still
> needs a way to retrieve the PASID for an mdev?

Yes.

As Kirti's suggestion, we removed this since there is currently no
consumer yet. We will bring it back together with the real consumer.

https://lkml.org/lkml/2018/11/16/124

Best regards,
Lu Baolu

> 
> Thanks,
> Jean
> 
>> Do we have
>> any consumers of this code yet?  Theoretically I think a vfio-pci-like
>> meta driver could be written as an mdev vendor driver with this support
>> (restricted to type1 iommu use cases).  Thanks,
>>
>> Alex
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
  2019-02-14 20:14 ` [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Alex Williamson
  2019-02-15 18:46     ` Jean-Philippe Brucker
@ 2019-02-19  2:49   ` Lu Baolu
  2019-02-20  5:09   ` Liu, Yi L
  2019-02-22  0:56   ` Lu Baolu
  3 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-19  2:49 UTC (permalink / raw)
  To: Alex Williamson, Kirti Wankhede
  Cc: baolu.lu, Joerg Roedel, David Woodhouse, ashok.raj,
	sanjay.k.kumar, jacob.jun.pan, kevin.tian, Jean-Philippe Brucker,
	yi.l.liu, yi.y.sun, peterx, tiwei.bie, xin.zeng, iommu, kvm,
	linux-kernel

Hi Kirti,

On 2/15/19 4:14 AM, Alex Williamson wrote:
> On Wed, 13 Feb 2019 12:02:52 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
> 
>> Hi,
>>
>> The Mediate Device is a framework for fine-grained physical device
>> sharing across the isolated domains. Currently the mdev framework
>> is designed to be independent of the platform IOMMU support. As the
>> result, the DMA isolation relies on the mdev parent device in a
>> vendor specific way.
>>
>> There are several cases where a mediated device could be protected
>> and isolated by the platform IOMMU. For example, Intel vt-d rev3.0
>> [1] introduces a new translation mode called 'scalable mode', which
>> enables PASID-granular translations. The vt-d scalable mode is the
>> key ingredient for Scalable I/O Virtualization [2] [3] which allows
>> sharing a device in minimal possible granularity (ADI - Assignable
>> Device Interface).
>>
>> A mediated device backed by an ADI could be protected and isolated
>> by the IOMMU since 1) the parent device supports tagging an unique
>> PASID to all DMA traffic out of the mediated device; and 2) the DMA
>> translation unit (IOMMU) supports the PASID granular translation.
>> We can apply IOMMU protection and isolation to this kind of devices
>> just as what we are doing with an assignable PCI device.
>>
>> In order to distinguish the IOMMU-capable mediated devices from those
>> which still need to rely on parent devices, this patch set adds one
>> new member in struct mdev_device.
>>
>> * iommu_device
>>    - This, if set, indicates that the mediated device could
>>      be fully isolated and protected by IOMMU via attaching
>>      an iommu domain to this device. If empty, it indicates
>>      using vendor defined isolation.
>>
>> Below helpers are added to set and get above iommu device in mdev core
>> implementation.
>>
>> * mdev_set/get_iommu_device(dev, iommu_device)
>>    - Set or get the iommu device which represents this mdev
>>      in IOMMU's device scope. Drivers don't need to set the
>>      iommu device if it uses vendor defined isolation.
>>
>> The mdev parent device driver could opt-in that the mdev could be
>> fully isolated and protected by the IOMMU when the mdev is being
>> created by invoking mdev_set_iommu_device() in its @create().
>>
>> In the vfio_iommu_type1_attach_group(), a domain allocated through
>> iommu_domain_alloc() will be attached to the mdev iommu device if
>> an iommu device has been set. Otherwise, the dummy external domain
>> will be used and all the DMA isolation and protection are routed to
>> parent driver as the result.
>>
>> On IOMMU side, a basic requirement is allowing to attach multiple
>> domains to a PCI device if the device advertises the capability
>> and the IOMMU hardware supports finer granularity translations than
>> the normal PCI Source ID based translation.
>>
>> As the result, a PCI device could work in two modes: normal mode
>> and auxiliary mode. In the normal mode, a pci device could be
>> isolated in the Source ID granularity; the pci device itself could
>> be assigned to a user application by attaching a single domain
>> to it. In the auxiliary mode, a pci device could be isolated in
>> finer granularity, hence subsets of the device could be assigned
>> to different user level application by attaching a different domain
>> to each subset.
>>
>> Below APIs are introduced in iommu generic layer for aux-domain
>> purpose:
>>
>> * iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
>>    - Check whether both IOMMU and device support IOMMU aux
>>      domain feature. Below aux-domain specific interfaces
>>      are available only after this returns true.
>>
>> * iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_AUX)
>>    - Enable/disable device specific aux-domain feature.
>>
>> * iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
>>    - Check whether the aux domain specific feature enabled or
>>      not.
>>
>> * iommu_aux_attach_device(domain, dev)
>>    - Attaches @domain to @dev in the auxiliary mode. Multiple
>>      domains could be attached to a single device in the
>>      auxiliary mode with each domain representing an isolated
>>      address space for an assignable subset of the device.
>>
>> * iommu_aux_detach_device(domain, dev)
>>    - Detach @domain which has been attached to @dev in the
>>      auxiliary mode.
>>
>> * iommu_aux_get_pasid(domain, dev)
>>    - Return ID used for finer-granularity DMA translation.
>>      For the Intel Scalable IOV usage model, this will be
>>      a PASID. The device which supports Scalable IOV needs
>>      to write this ID to the device register so that DMA
>>      requests could be tagged with a right PASID prefix.
>>
>> In order for the ease of discussion, sometimes we call "a domain in
>> auxiliary mode' or simply 'an auxiliary domain' when a domain is
>> attached to a device for finer granularity translations. But we need
>> to keep in mind that this doesn't mean there is a differnt domain
>> type. A same domain could be bound to a device for Source ID based
>> translation, and bound to another device for finer granularity
>> translation at the same time.
>>
>> This patch series extends both IOMMU and vfio components to support
>> mdev device passing through when it could be isolated and protected
>> by the IOMMU units. The first part of this series (PATCH 1/09~6/09)
>> adds the interfaces and implementation of the multiple domains per
>> device. The second part (PATCH 7/09~9/09) adds the iommu device
>> attribute to each mdev, determines isolation type according to the
>> existence of an iommu device when attaching group in vfio type1 iommu
>> module, and attaches the domain to iommu aware mediated devices.
>>
>> References:
>> [1] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
>> [2] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
>> [3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
>>
>> Best regards,
>> Lu Baolu
>>
>> Change log:
>>    v5->v6:
> 
> This looks pretty reasonable with Jean-Philippe's nit fixups.  Where do
> we go from here?  I think we need an ack from Kirti since they have an
> interest here.  Presumably this looks ok to the ARM folks.  Do we have

Any comments?

Best regards,
Lu Baolu

> any consumers of this code yet?  Theoretically I think a vfio-pci-like
> meta driver could be written as an mdev vendor driver with this support
> (restricted to type1 iommu use cases).  Thanks,
> 
> Alex
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
  2019-02-14 20:14 ` [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Alex Williamson
  2019-02-15 18:46     ` Jean-Philippe Brucker
  2019-02-19  2:49   ` Lu Baolu
@ 2019-02-20  5:09   ` Liu, Yi L
  2019-02-22  0:56   ` Lu Baolu
  3 siblings, 0 replies; 19+ messages in thread
From: Liu, Yi L @ 2019-02-20  5:09 UTC (permalink / raw)
  To: Alex Williamson, Lu Baolu
  Cc: Joerg Roedel, David Woodhouse, Kirti Wankhede, Raj, Ashok, Kumar,
	Sanjay K, Pan, Jacob jun, Tian, Kevin, Jean-Philippe Brucker,
	Sun, Yi Y, peterx, Bie, Tiwei, Zeng, Xin, iommu, kvm,
	linux-kernel

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Friday, February 15, 2019 4:15 AM
> To: Lu Baolu <baolu.lu@linux.intel.com>
> Subject: Re: [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
> 
> On Wed, 13 Feb 2019 12:02:52 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
> 
> > Hi,
> >
> > The Mediate Device is a framework for fine-grained physical device
> > sharing across the isolated domains. Currently the mdev framework is
> > designed to be independent of the platform IOMMU support. As the
> > result, the DMA isolation relies on the mdev parent device in a vendor
> > specific way.
> >
> > There are several cases where a mediated device could be protected and
> > isolated by the platform IOMMU. For example, Intel vt-d rev3.0 [1]
> > introduces a new translation mode called 'scalable mode', which
> > enables PASID-granular translations. The vt-d scalable mode is the key
> > ingredient for Scalable I/O Virtualization [2] [3] which allows
> > sharing a device in minimal possible granularity (ADI - Assignable
> > Device Interface).
> >
> > A mediated device backed by an ADI could be protected and isolated by
> > the IOMMU since 1) the parent device supports tagging an unique PASID
> > to all DMA traffic out of the mediated device; and 2) the DMA
> > translation unit (IOMMU) supports the PASID granular translation.
> > We can apply IOMMU protection and isolation to this kind of devices
> > just as what we are doing with an assignable PCI device.
> >
> > In order to distinguish the IOMMU-capable mediated devices from those
> > which still need to rely on parent devices, this patch set adds one
> > new member in struct mdev_device.
> >
> > * iommu_device
> >   - This, if set, indicates that the mediated device could
> >     be fully isolated and protected by IOMMU via attaching
> >     an iommu domain to this device. If empty, it indicates
> >     using vendor defined isolation.
> >
> > Below helpers are added to set and get above iommu device in mdev core
> > implementation.
> >
> > * mdev_set/get_iommu_device(dev, iommu_device)
> >   - Set or get the iommu device which represents this mdev
> >     in IOMMU's device scope. Drivers don't need to set the
> >     iommu device if it uses vendor defined isolation.
> >
> > The mdev parent device driver could opt-in that the mdev could be
> > fully isolated and protected by the IOMMU when the mdev is being
> > created by invoking mdev_set_iommu_device() in its @create().
> >
> > In the vfio_iommu_type1_attach_group(), a domain allocated through
> > iommu_domain_alloc() will be attached to the mdev iommu device if an
> > iommu device has been set. Otherwise, the dummy external domain will
> > be used and all the DMA isolation and protection are routed to parent
> > driver as the result.
> >
> > On IOMMU side, a basic requirement is allowing to attach multiple
> > domains to a PCI device if the device advertises the capability and
> > the IOMMU hardware supports finer granularity translations than the
> > normal PCI Source ID based translation.
> >
> > As the result, a PCI device could work in two modes: normal mode and
> > auxiliary mode. In the normal mode, a pci device could be isolated in
> > the Source ID granularity; the pci device itself could be assigned to
> > a user application by attaching a single domain to it. In the
> > auxiliary mode, a pci device could be isolated in finer granularity,
> > hence subsets of the device could be assigned to different user level
> > application by attaching a different domain to each subset.
> >
> > Below APIs are introduced in iommu generic layer for aux-domain
> > purpose:
> >
> > * iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
> >   - Check whether both IOMMU and device support IOMMU aux
> >     domain feature. Below aux-domain specific interfaces
> >     are available only after this returns true.
> >
> > * iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_AUX)
> >   - Enable/disable device specific aux-domain feature.
> >
> > * iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
> >   - Check whether the aux domain specific feature enabled or
> >     not.
> >
> > * iommu_aux_attach_device(domain, dev)
> >   - Attaches @domain to @dev in the auxiliary mode. Multiple
> >     domains could be attached to a single device in the
> >     auxiliary mode with each domain representing an isolated
> >     address space for an assignable subset of the device.
> >
> > * iommu_aux_detach_device(domain, dev)
> >   - Detach @domain which has been attached to @dev in the
> >     auxiliary mode.
> >
> > * iommu_aux_get_pasid(domain, dev)
> >   - Return ID used for finer-granularity DMA translation.
> >     For the Intel Scalable IOV usage model, this will be
> >     a PASID. The device which supports Scalable IOV needs
> >     to write this ID to the device register so that DMA
> >     requests could be tagged with a right PASID prefix.
> >
> > In order for the ease of discussion, sometimes we call "a domain in
> > auxiliary mode' or simply 'an auxiliary domain' when a domain is
> > attached to a device for finer granularity translations. But we need
> > to keep in mind that this doesn't mean there is a differnt domain
> > type. A same domain could be bound to a device for Source ID based
> > translation, and bound to another device for finer granularity
> > translation at the same time.
> >
> > This patch series extends both IOMMU and vfio components to support
> > mdev device passing through when it could be isolated and protected by
> > the IOMMU units. The first part of this series (PATCH 1/09~6/09) adds
> > the interfaces and implementation of the multiple domains per device.
> > The second part (PATCH 7/09~9/09) adds the iommu device attribute to
> > each mdev, determines isolation type according to the existence of an
> > iommu device when attaching group in vfio type1 iommu module, and
> > attaches the domain to iommu aware mediated devices.
> >
> > References:
> > [1]
> > https://software.intel.com/en-us/download/intel-virtualization-technol
> > ogy-for-directed-io-architecture-specification
> > [2]
> > https://software.intel.com/en-us/download/intel-scalable-io-virtualiza
> > tion-technical-specification [3]
> > https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
> >
> > Best regards,
> > Lu Baolu
> >
> > Change log:
> >   v5->v6:
> 
> This looks pretty reasonable with Jean-Philippe's nit fixups.  Where do we go from
> here?  I think we need an ack from Kirti since they have an interest here.  Presumably
> this looks ok to the ARM folks.  Do we have any consumers of this code yet?
> Theoretically I think a vfio-pci-like meta driver could be written as an mdev vendor
> driver with this support (restricted to type1 iommu use cases).  Thanks,

Hi Alex,

I'll do some work on the vfio-pci-like meta driver to be a sample consumer of
the vfio mdev extensions in this patchset. Will send it later.

Thanks,
Yi Liu

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device
  2019-02-14 20:14 ` [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Alex Williamson
                     ` (2 preceding siblings ...)
  2019-02-20  5:09   ` Liu, Yi L
@ 2019-02-22  0:56   ` Lu Baolu
  3 siblings, 0 replies; 19+ messages in thread
From: Lu Baolu @ 2019-02-22  0:56 UTC (permalink / raw)
  To: Alex Williamson
  Cc: baolu.lu, Joerg Roedel, David Woodhouse, Kirti Wankhede,
	ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian,
	Jean-Philippe Brucker, yi.l.liu, yi.y.sun, peterx, tiwei.bie,
	xin.zeng, iommu, kvm, linux-kernel

Hi,

On 2/15/19 4:14 AM, Alex Williamson wrote:
> On Wed, 13 Feb 2019 12:02:52 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
> 
>> Hi,
>>
>> The Mediate Device is a framework for fine-grained physical device
>> sharing across the isolated domains. Currently the mdev framework
>> is designed to be independent of the platform IOMMU support. As the
>> result, the DMA isolation relies on the mdev parent device in a
>> vendor specific way.
>>
>> There are several cases where a mediated device could be protected
>> and isolated by the platform IOMMU. For example, Intel vt-d rev3.0
>> [1] introduces a new translation mode called 'scalable mode', which
>> enables PASID-granular translations. The vt-d scalable mode is the
>> key ingredient for Scalable I/O Virtualization [2] [3] which allows
>> sharing a device in minimal possible granularity (ADI - Assignable
>> Device Interface).
>>
>> A mediated device backed by an ADI could be protected and isolated
>> by the IOMMU since 1) the parent device supports tagging an unique
>> PASID to all DMA traffic out of the mediated device; and 2) the DMA
>> translation unit (IOMMU) supports the PASID granular translation.
>> We can apply IOMMU protection and isolation to this kind of devices
>> just as what we are doing with an assignable PCI device.
>>
>> In order to distinguish the IOMMU-capable mediated devices from those
>> which still need to rely on parent devices, this patch set adds one
>> new member in struct mdev_device.
>>
>> * iommu_device
>>    - This, if set, indicates that the mediated device could
>>      be fully isolated and protected by IOMMU via attaching
>>      an iommu domain to this device. If empty, it indicates
>>      using vendor defined isolation.
>>
>> Below helpers are added to set and get above iommu device in mdev core
>> implementation.
>>
>> * mdev_set/get_iommu_device(dev, iommu_device)
>>    - Set or get the iommu device which represents this mdev
>>      in IOMMU's device scope. Drivers don't need to set the
>>      iommu device if it uses vendor defined isolation.
>>
>> The mdev parent device driver could opt-in that the mdev could be
>> fully isolated and protected by the IOMMU when the mdev is being
>> created by invoking mdev_set_iommu_device() in its @create().
>>
>> In the vfio_iommu_type1_attach_group(), a domain allocated through
>> iommu_domain_alloc() will be attached to the mdev iommu device if
>> an iommu device has been set. Otherwise, the dummy external domain
>> will be used and all the DMA isolation and protection are routed to
>> parent driver as the result.
>>
>> On IOMMU side, a basic requirement is allowing to attach multiple
>> domains to a PCI device if the device advertises the capability
>> and the IOMMU hardware supports finer granularity translations than
>> the normal PCI Source ID based translation.
>>
>> As the result, a PCI device could work in two modes: normal mode
>> and auxiliary mode. In the normal mode, a pci device could be
>> isolated in the Source ID granularity; the pci device itself could
>> be assigned to a user application by attaching a single domain
>> to it. In the auxiliary mode, a pci device could be isolated in
>> finer granularity, hence subsets of the device could be assigned
>> to different user level application by attaching a different domain
>> to each subset.
>>
>> Below APIs are introduced in iommu generic layer for aux-domain
>> purpose:
>>
>> * iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
>>    - Check whether both IOMMU and device support IOMMU aux
>>      domain feature. Below aux-domain specific interfaces
>>      are available only after this returns true.
>>
>> * iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_AUX)
>>    - Enable/disable device specific aux-domain feature.
>>
>> * iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)
>>    - Check whether the aux domain specific feature enabled or
>>      not.
>>
>> * iommu_aux_attach_device(domain, dev)
>>    - Attaches @domain to @dev in the auxiliary mode. Multiple
>>      domains could be attached to a single device in the
>>      auxiliary mode with each domain representing an isolated
>>      address space for an assignable subset of the device.
>>
>> * iommu_aux_detach_device(domain, dev)
>>    - Detach @domain which has been attached to @dev in the
>>      auxiliary mode.
>>
>> * iommu_aux_get_pasid(domain, dev)
>>    - Return ID used for finer-granularity DMA translation.
>>      For the Intel Scalable IOV usage model, this will be
>>      a PASID. The device which supports Scalable IOV needs
>>      to write this ID to the device register so that DMA
>>      requests could be tagged with a right PASID prefix.
>>
>> In order for the ease of discussion, sometimes we call "a domain in
>> auxiliary mode' or simply 'an auxiliary domain' when a domain is
>> attached to a device for finer granularity translations. But we need
>> to keep in mind that this doesn't mean there is a differnt domain
>> type. A same domain could be bound to a device for Source ID based
>> translation, and bound to another device for finer granularity
>> translation at the same time.
>>
>> This patch series extends both IOMMU and vfio components to support
>> mdev device passing through when it could be isolated and protected
>> by the IOMMU units. The first part of this series (PATCH 1/09~6/09)
>> adds the interfaces and implementation of the multiple domains per
>> device. The second part (PATCH 7/09~9/09) adds the iommu device
>> attribute to each mdev, determines isolation type according to the
>> existence of an iommu device when attaching group in vfio type1 iommu
>> module, and attaches the domain to iommu aware mediated devices.
>>
>> References:
>> [1] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
>> [2] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
>> [3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
>>
>> Best regards,
>> Lu Baolu
>>
>> Change log:
>>    v5->v6:
> 
> This looks pretty reasonable with Jean-Philippe's nit fixups.  Where do
> we go from here?  I think we need an ack from Kirti since they have an
> interest here.  Presumably this looks ok to the ARM folks.  Do we have
> any consumers of this code yet?  Theoretically I think a vfio-pci-like
> meta driver could be written as an mdev vendor driver with this support
> (restricted to type1 iommu use cases).  Thanks,
> 

I saw Jean's device driver API for SVA which is based on the work in
this patch series. Let me update this patch series with Jean's comments
here so that people could always base on the latest code.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-02-22  1:01 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-13  4:02 [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Lu Baolu
2019-02-13  4:02 ` [PATCH v6 1/9] iommu: Add APIs for multiple domains per device Lu Baolu
2019-02-13 11:55   ` Jean-Philippe Brucker
2019-02-14  1:34     ` Lu Baolu
2019-02-13  4:02 ` [PATCH v6 2/9] iommu/vt-d: Move enable pasid out of CONFIG_INTEL_IOMMU_SVM Lu Baolu
2019-02-13  4:02 ` [PATCH v6 3/9] iommu/vt-d: Add per-device IOMMU feature ops entries Lu Baolu
2019-02-13  4:02 ` [PATCH v6 4/9] iommu/vt-d: Move common code out of iommu_attch_device() Lu Baolu
2019-02-13  4:02 ` [PATCH v6 5/9] iommu/vt-d: Aux-domain specific domain attach/detach Lu Baolu
2019-02-13  4:02 ` [PATCH v6 6/9] iommu/vt-d: Return ID associated with an auxiliary domain Lu Baolu
2019-02-13  4:02 ` [PATCH v6 7/9] vfio/mdev: Add iommu related member in mdev_device Lu Baolu
2019-02-13  4:03 ` [PATCH v6 8/9] vfio/type1: Add domain at(de)taching group helpers Lu Baolu
2019-02-13  4:03 ` [PATCH v6 9/9] vfio/type1: Handle different mdev isolation type Lu Baolu
2019-02-14 20:14 ` [PATCH v6 0/9] vfio/mdev: IOMMU aware mediated device Alex Williamson
2019-02-15 18:46   ` Jean-Philippe Brucker
2019-02-15 18:46     ` Jean-Philippe Brucker
2019-02-19  2:46     ` Lu Baolu
2019-02-19  2:49   ` Lu Baolu
2019-02-20  5:09   ` Liu, Yi L
2019-02-22  0:56   ` Lu Baolu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.