| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 31 Jul 2022 21:51:36 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Yi Liu <yi.l.liu@...el.com>, Joerg Roedel <joro@...tes.org>,
Jason Gunthorpe <jgg@...dia.com>,
Christoph Hellwig <hch@...radead.org>,
Kevin Tian <kevin.tian@...el.com>,
Ashok Raj <ashok.raj@...el.com>, Will Deacon <will@...nel.org>,
Robin Murphy <robin.murphy@....com>,
Jean-Philippe Brucker <jean-philippe@...aro.com>,
Dave Jiang <dave.jiang@...el.com>,
Vinod Koul <vkoul@...nel.org>
Cc: baolu.lu@...ux.intel.com, Eric Auger <eric.auger@...hat.com>,
Jacob jun Pan <jacob.jun.pan@...el.com>,
Zhangfei Gao <zhangfei.gao@...aro.org>,
Zhu Tony <tony.zhu@...el.com>, iommu@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 08/12] iommu/sva: Refactoring
iommu_sva_bind/unbind_device()
On 2022/7/31 20:55, Yi Liu wrote:
> On 2022/7/5 13:07, Lu Baolu wrote:
>> The existing iommu SVA interfaces are implemented by calling the SVA
>> specific iommu ops provided by the IOMMU drivers. There's no need for
>> any SVA specific ops in iommu_ops vector anymore as we can achieve
>> this through the generic attach/detach_dev_pasid domain ops.
>>
>> This refactors the IOMMU SVA interfaces implementation by using the
>> set/block_pasid_dev ops and align them with the concept of the SVA
>> iommu domain. Put the new SVA code in the sva related file in order
>> to make it self-contained.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
>> Tested-by: Zhangfei Gao <zhangfei.gao@...aro.org>
>> Tested-by: Tony Zhu <tony.zhu@...el.com>
>> ---
>> include/linux/iommu.h | 67 +++++++++++--------
>> drivers/iommu/iommu-sva-lib.c | 98 ++++++++++++++++++++++++++++
>> drivers/iommu/iommu.c | 119 ++++++++--------------------------
>> 3 files changed, 165 insertions(+), 119 deletions(-)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 42f0418dc22c..f59b0ecd3995 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -39,7 +39,6 @@ struct device;
>> struct iommu_domain;
>> struct iommu_domain_ops;
>> struct notifier_block;
>> -struct iommu_sva;
>> struct iommu_fault_event;
>> struct iommu_dma_cookie;
>> @@ -57,6 +56,14 @@ struct iommu_domain_geometry {
>> bool force_aperture; /* DMA only allowed in mappable
>> range? */
>> };
>> +/**
>> + * struct iommu_sva - handle to a device-mm bond
>> + */
>> +struct iommu_sva {
>> + struct device *dev;
>> + refcount_t users;
>> +};
>> +
>> /* Domain feature flags */
>> #define __IOMMU_DOMAIN_PAGING (1U << 0) /* Support for
>> iommu_map/unmap */
>> #define __IOMMU_DOMAIN_DMA_API (1U << 1) /* Domain for use in
>> DMA-API
>> @@ -105,6 +112,7 @@ struct iommu_domain {
>> };
>> struct { /* IOMMU_DOMAIN_SVA */
>> struct mm_struct *mm;
>> + struct iommu_sva bond;
>> };
>> };
>> };
>> @@ -638,13 +646,6 @@ struct iommu_fwspec {
>> /* ATS is supported */
>> #define IOMMU_FWSPEC_PCI_RC_ATS (1 << 0)
>> -/**
>> - * struct iommu_sva - handle to a device-mm bond
>> - */
>> -struct iommu_sva {
>> - struct device *dev;
>> -};
>> -
>> int iommu_fwspec_init(struct device *dev, struct fwnode_handle
>> *iommu_fwnode,
>> const struct iommu_ops *ops);
>> void iommu_fwspec_free(struct device *dev);
>> @@ -685,11 +686,6 @@ 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);
>> -struct iommu_sva *iommu_sva_bind_device(struct device *dev,
>> - struct mm_struct *mm);
>> -void iommu_sva_unbind_device(struct iommu_sva *handle);
>> -u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>> -
>> int iommu_device_use_default_domain(struct device *dev);
>> void iommu_device_unuse_default_domain(struct device *dev);
>> @@ -703,6 +699,8 @@ int iommu_attach_device_pasid(struct iommu_domain
>> *domain, struct device *dev,
>> ioasid_t pasid);
>> void iommu_detach_device_pasid(struct iommu_domain *domain, struct
>> device *dev,
>> ioasid_t pasid);
>> +struct iommu_domain *
>> +iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid);
>> #else /* CONFIG_IOMMU_API */
>> struct iommu_ops {};
>> @@ -1033,21 +1031,6 @@ iommu_dev_disable_feature(struct device *dev,
>> enum iommu_dev_features feat)
>> return -ENODEV;
>> }
>> -static inline struct iommu_sva *
>> -iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
>> -{
>> - return NULL;
>> -}
>> -
>> -static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
>> -{
>> -}
>> -
>> -static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
>> -{
>> - return IOMMU_PASID_INVALID;
>> -}
>> -
>> static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct
>> device *dev)
>> {
>> return NULL;
>> @@ -1093,6 +1076,12 @@ static inline void
>> iommu_detach_device_pasid(struct iommu_domain *domain,
>> struct device *dev, ioasid_t pasid)
>> {
>> }
>> +
>> +static inline struct iommu_domain *
>> +iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid)
>> +{
>> + return NULL;
>> +}
>> #endif /* CONFIG_IOMMU_API */
>> /**
>> @@ -1118,4 +1107,26 @@ void iommu_debugfs_setup(void);
>> static inline void iommu_debugfs_setup(void) {}
>> #endif
>> +#ifdef CONFIG_IOMMU_SVA
>> +struct iommu_sva *iommu_sva_bind_device(struct device *dev,
>> + struct mm_struct *mm);
>> +void iommu_sva_unbind_device(struct iommu_sva *handle);
>> +u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>> +#else
>> +static inline struct iommu_sva *
>> +iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
>> +{
>> + return NULL;
>> +}
>> +
>> +static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
>> +{
>> +}
>> +
>> +static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
>> +{
>> + return IOMMU_PASID_INVALID;
>> +}
>> +#endif /* CONFIG_IOMMU_SVA */
>> +
>> #endif /* __LINUX_IOMMU_H */
>> diff --git a/drivers/iommu/iommu-sva-lib.c
>> b/drivers/iommu/iommu-sva-lib.c
>> index 106506143896..751366980232 100644
>> --- a/drivers/iommu/iommu-sva-lib.c
>> +++ b/drivers/iommu/iommu-sva-lib.c
>> @@ -4,6 +4,7 @@
>> */
>> #include <linux/mutex.h>
>> #include <linux/sched/mm.h>
>> +#include <linux/iommu.h>
>> #include "iommu-sva-lib.h"
>> @@ -69,3 +70,100 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid)
>> return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
>> }
>> EXPORT_SYMBOL_GPL(iommu_sva_find);
>> +
>> +/**
>> + * iommu_sva_bind_device() - Bind a process address space to a device
>> + * @dev: the device
>> + * @mm: the mm to bind, caller must hold a reference to mm_users
>> + *
>> + * Create a bond between device and address space, allowing the
>> device to access
>> + * the mm using the returned PASID. If a bond already exists between
>> @device and
>> + * @mm, it is returned and an additional reference is taken. Caller
>> must call
>> + * iommu_sva_unbind_device() to release each reference.
>> + *
>> + * iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) must be called
>> first, to
>> + * initialize the required SVA features.
>> + *
>> + * On error, returns an ERR_PTR value.
>> + */
>> +struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct
>> mm_struct *mm)
>> +{
>> + struct iommu_domain *domain;
>> + ioasid_t max_pasids;
>> + int ret = -EINVAL;
>> +
>> + max_pasids = dev->iommu->max_pasids;
>> + if (!max_pasids)
>> + return ERR_PTR(-EOPNOTSUPP);
>> +
>> + /* Allocate mm->pasid if necessary. */
>> + ret = iommu_sva_alloc_pasid(mm, 1, max_pasids - 1);
>
> do we want to call mmgrab() before iomu_sva_alloc_pasid() to
> avoid using mm without any reference? In your current code,
> mmgrab() is called in iommu_sva_domain_alloc().
As the comment of this API states "caller must hold a reference to
mm_users".
Best regards,
baolu
Powered by blists - more mailing lists