[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190114112230.00002ec1@huawei.com>
Date: Mon, 14 Jan 2019 11:22:30 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Lu Baolu <baolu.lu@...ux.intel.com>
CC: Joerg Roedel <joro@...tes.org>,
David Woodhouse <dwmw2@...radead.org>,
Alex Williamson <alex.williamson@...hat.com>,
Kirti Wankhede <kwankhede@...dia.com>, <kevin.tian@...el.com>,
<ashok.raj@...el.com>, <tiwei.bie@...el.com>,
Jean-Philippe Brucker <jean-philippe.brucker@....com>,
<sanjay.k.kumar@...el.com>, <iommu@...ts.linux-foundation.org>,
<linux-kernel@...r.kernel.org>, <Zeng@...l.linuxfoundation.org>,
<yi.y.sun@...el.com>, <jacob.jun.pan@...el.com>,
<kvm@...r.kernel.org>
Subject: Re: [PATCH v5 1/8] iommu: Add APIs for multiple domains per device
On Thu, 10 Jan 2019 11:00:20 +0800
Lu Baolu <baolu.lu@...ux.intel.com> 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. 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_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@...el.com>
> Liu Yi L <yi.l.liu@...el.com>
> Ashok Raj <ashok.raj@...el.com>
> Sanjay Kumar <sanjay.k.kumar@...el.com>
> Jacob Pan <jacob.jun.pan@...ux.intel.com>
> Alex Williamson <alex.williamson@...hat.com>
> Jean-Philippe Brucker <jean-philippe.brucker@....com>
> Joerg Roedel <joro@...tes.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@...el.com>
> Cc: Jacob Pan <jacob.jun.pan@...ux.intel.com>
> Cc: Kevin Tian <kevin.tian@...el.com>
> Cc: Liu Yi L <yi.l.liu@...el.com>
> Suggested-by: Kevin Tian <kevin.tian@...el.com>
> Suggested-by: Jean-Philippe Brucker <jean-philippe.brucker@....com>
> Suggested-by: Joerg Roedel <jroedel@...e.de>
> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
One trivial comment inline.
> ---
> drivers/iommu/iommu.c | 80 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/iommu.h | 61 +++++++++++++++++++++++++++++++++
> 2 files changed, 141 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 3ed4db334341..9166b6145409 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2033,3 +2033,83 @@ 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);
> +
> +/*
> + * 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
when trying to re-attach. (perhaps?)
> + * 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..f4c3d2a2cc87 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,10 @@ 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.
> + * @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 +235,16 @@ 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);
> + 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 +431,13 @@ 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 feat);
> +int iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat);
> +int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat);
> +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 +722,41 @@ 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 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
Powered by blists - more mailing lists