* [PATCH v12 01/15] iommu: Introduce attach/detach_pasid_table API
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 15:14 ` kernel test robot
2020-11-16 10:43 ` [PATCH v12 02/15] iommu: Introduce bind/unbind_guest_msi Eric Auger
` (13 subsequent siblings)
14 siblings, 1 reply; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on the guest,
the physical IOMMU must be programmed to be consistent with
the guest mappings. If the physical IOMMU supports two
translation stages it makes sense to program guest mappings
onto the first stage/level (ARM/Intel terminology) while the host
owns the stage/level 2.
In that case, it is mandated to trap on guest configuration
settings and pass those to the physical iommu driver.
This patch adds a new API to the iommu subsystem that allows
to set/unset the pasid table information.
A generic iommu_pasid_table_config struct is introduced in
a new iommu.h uapi header. This is going to be used by the VFIO
user API.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v11 -> v12:
- add argsz, name the union
---
drivers/iommu/iommu.c | 67 ++++++++++++++++++++++++++++++++++++++
include/linux/iommu.h | 21 ++++++++++++
include/uapi/linux/iommu.h | 54 ++++++++++++++++++++++++++++++
3 files changed, 142 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b53446bb8c6b..b061bf4c3bb2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2171,6 +2171,73 @@ int iommu_uapi_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev
}
EXPORT_SYMBOL_GPL(iommu_uapi_sva_unbind_gpasid);
+int iommu_attach_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ if (unlikely(!domain->ops->attach_pasid_table))
+ return -ENODEV;
+
+ return domain->ops->attach_pasid_table(domain, cfg);
+}
+
+int iommu_uapi_attach_pasid_table(struct iommu_domain *domain,
+ void __user *uinfo)
+{
+ struct iommu_pasid_table_config pasid_table_data = { 0 };
+ u32 minsz;
+
+ if (unlikely(!domain->ops->attach_pasid_table))
+ return -ENODEV;
+
+ /*
+ * No new spaces can be added before the variable sized union, the
+ * minimum size is the offset to the union.
+ */
+ minsz = offsetof(struct iommu_pasid_table_config, vendor_data);
+
+ /* Copy minsz from user to get flags and argsz */
+ if (copy_from_user(&pasid_table_data, uinfo, minsz))
+ return -EFAULT;
+
+ /* Fields before the variable size union are mandatory */
+ if (pasid_table_data.argsz < minsz)
+ return -EINVAL;
+
+ /* PASID and address granu require additional info beyond minsz */
+ if (pasid_table_data.version != PASID_TABLE_CFG_VERSION_1)
+ return -EINVAL;
+ if (pasid_table_data.format == IOMMU_PASID_FORMAT_SMMUV3 &&
+ pasid_table_data.argsz <
+ offsetofend(struct iommu_pasid_table_config, vendor_data.smmuv3))
+ return -EINVAL;
+
+ /*
+ * User might be using a newer UAPI header which has a larger data
+ * size, we shall support the existing flags within the current
+ * size. Copy the remaining user data _after_ minsz but not more
+ * than the current kernel supported size.
+ */
+ if (copy_from_user((void *)&pasid_table_data + minsz, uinfo + minsz,
+ min_t(u32, pasid_table_data.argsz, sizeof(pasid_table_data)) - minsz))
+ return -EFAULT;
+
+ /* Now the argsz is validated, check the content */
+ if (pasid_table_data.config < 1 && pasid_table_data.config > 3)
+ return -EINVAL;
+
+ return domain->ops->attach_pasid_table(domain, &pasid_table_data);
+}
+EXPORT_SYMBOL_GPL(iommu_uapi_attach_pasid_table);
+
+void iommu_detach_pasid_table(struct iommu_domain *domain)
+{
+ if (unlikely(!domain->ops->detach_pasid_table))
+ return;
+
+ domain->ops->detach_pasid_table(domain);
+}
+EXPORT_SYMBOL_GPL(iommu_detach_pasid_table);
+
static void __iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
{
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b95a6f8db6ff..464fcbecf841 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -223,6 +223,8 @@ struct iommu_iotlb_gather {
* @cache_invalidate: invalidate translation caches
* @sva_bind_gpasid: bind guest pasid and mm
* @sva_unbind_gpasid: unbind guest pasid and mm
+ * @attach_pasid_table: attach a pasid table
+ * @detach_pasid_table: detach the pasid table
* @def_domain_type: device default domain type, return value:
* - IOMMU_DOMAIN_IDENTITY: must use an identity domain
* - IOMMU_DOMAIN_DMA: must use a dma domain
@@ -287,6 +289,9 @@ struct iommu_ops {
void *drvdata);
void (*sva_unbind)(struct iommu_sva *handle);
u32 (*sva_get_pasid)(struct iommu_sva *handle);
+ int (*attach_pasid_table)(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg);
+ void (*detach_pasid_table)(struct iommu_domain *domain);
int (*page_response)(struct device *dev,
struct iommu_fault_event *evt,
@@ -434,6 +439,11 @@ extern int iommu_uapi_sva_unbind_gpasid(struct iommu_domain *domain,
struct device *dev, void __user *udata);
extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain,
struct device *dev, ioasid_t pasid);
+extern int iommu_attach_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg);
+extern int iommu_uapi_attach_pasid_table(struct iommu_domain *domain,
+ void __user *udata);
+extern void iommu_detach_pasid_table(struct iommu_domain *domain);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -639,6 +649,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
void iommu_sva_unbind_device(struct iommu_sva *handle);
u32 iommu_sva_get_pasid(struct iommu_sva *handle);
+
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
@@ -1020,6 +1031,16 @@ iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
return -ENODEV;
}
+static inline
+int iommu_attach_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ return -ENODEV;
+}
+
+static inline
+void iommu_detach_pasid_table(struct iommu_domain *domain) {}
+
static inline struct iommu_sva *
iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
{
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index e1d9e75f2c94..082d758dd016 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -338,4 +338,58 @@ struct iommu_gpasid_bind_data {
} vendor;
};
+/**
+ * struct iommu_pasid_smmuv3 - ARM SMMUv3 Stream Table Entry stage 1 related
+ * information
+ * @version: API version of this structure
+ * @s1fmt: STE s1fmt (format of the CD table: single CD, linear table
+ * or 2-level table)
+ * @s1dss: STE s1dss (specifies the behavior when @pasid_bits != 0
+ * and no PASID is passed along with the incoming transaction)
+ * @padding: reserved for future use (should be zero)
+ *
+ * The PASID table is referred to as the Context Descriptor (CD) table on ARM
+ * SMMUv3. Please refer to the ARM SMMU 3.x spec (ARM IHI 0070A) for full
+ * details.
+ */
+struct iommu_pasid_smmuv3 {
+#define PASID_TABLE_SMMUV3_CFG_VERSION_1 1
+ __u32 version;
+ __u8 s1fmt;
+ __u8 s1dss;
+ __u8 padding[2];
+};
+
+/**
+ * struct iommu_pasid_table_config - PASID table data used to bind guest PASID
+ * table to the host IOMMU
+ * @argsz: User filled size of this data
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ * @base_ptr: guest physical address of the PASID table
+ * @pasid_bits: number of PASID bits used in the PASID table
+ * @config: indicates whether the guest translation stage must
+ * be translated, bypassed or aborted.
+ * @padding: reserved for future use (should be zero)
+ * @vendor_data.smmuv3: table information when @format is
+ * %IOMMU_PASID_FORMAT_SMMUV3
+ */
+struct iommu_pasid_table_config {
+ __u32 argsz;
+#define PASID_TABLE_CFG_VERSION_1 1
+ __u32 version;
+#define IOMMU_PASID_FORMAT_SMMUV3 1
+ __u32 format;
+ __u64 base_ptr;
+ __u8 pasid_bits;
+#define IOMMU_PASID_CONFIG_TRANSLATE 1
+#define IOMMU_PASID_CONFIG_BYPASS 2
+#define IOMMU_PASID_CONFIG_ABORT 3
+ __u8 config;
+ __u8 padding[2];
+ union {
+ struct iommu_pasid_smmuv3 smmuv3;
+ } vendor_data;
+};
+
#endif /* _UAPI_IOMMU_H */
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v12 01/15] iommu: Introduce attach/detach_pasid_table API
2020-11-16 10:43 ` [PATCH v12 01/15] iommu: Introduce attach/detach_pasid_table API Eric Auger
@ 2020-11-16 15:14 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2020-11-16 15:14 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: kbuild-all, clang-built-linux
[-- Attachment #1: Type: text/plain, Size: 4070 bytes --]
Hi Eric,
I love your patch! Perhaps something to improve:
[auto build test WARNING on iommu/next]
[also build test WARNING on linus/master v5.10-rc4 next-20201116]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Eric-Auger/SMMUv3-Nested-Stage-Setup-IOMMU-part/20201116-185039
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: arm64-randconfig-r034-20201115 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c044709b8fbea2a9a375e4173a6bd735f6866c0c)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/54be9a9e014a566f9c7640da201c24cfb1eda06e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eric-Auger/SMMUv3-Nested-Stage-Setup-IOMMU-part/20201116-185039
git checkout 54be9a9e014a566f9c7640da201c24cfb1eda06e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/iommu/iommu.c:2225:34: warning: overlapping comparisons always evaluate to false [-Wtautological-overlap-compare]
if (pasid_table_data.config < 1 && pasid_table_data.config > 3)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
vim +2225 drivers/iommu/iommu.c
2182
2183 int iommu_uapi_attach_pasid_table(struct iommu_domain *domain,
2184 void __user *uinfo)
2185 {
2186 struct iommu_pasid_table_config pasid_table_data = { 0 };
2187 u32 minsz;
2188
2189 if (unlikely(!domain->ops->attach_pasid_table))
2190 return -ENODEV;
2191
2192 /*
2193 * No new spaces can be added before the variable sized union, the
2194 * minimum size is the offset to the union.
2195 */
2196 minsz = offsetof(struct iommu_pasid_table_config, vendor_data);
2197
2198 /* Copy minsz from user to get flags and argsz */
2199 if (copy_from_user(&pasid_table_data, uinfo, minsz))
2200 return -EFAULT;
2201
2202 /* Fields before the variable size union are mandatory */
2203 if (pasid_table_data.argsz < minsz)
2204 return -EINVAL;
2205
2206 /* PASID and address granu require additional info beyond minsz */
2207 if (pasid_table_data.version != PASID_TABLE_CFG_VERSION_1)
2208 return -EINVAL;
2209 if (pasid_table_data.format == IOMMU_PASID_FORMAT_SMMUV3 &&
2210 pasid_table_data.argsz <
2211 offsetofend(struct iommu_pasid_table_config, vendor_data.smmuv3))
2212 return -EINVAL;
2213
2214 /*
2215 * User might be using a newer UAPI header which has a larger data
2216 * size, we shall support the existing flags within the current
2217 * size. Copy the remaining user data _after_ minsz but not more
2218 * than the current kernel supported size.
2219 */
2220 if (copy_from_user((void *)&pasid_table_data + minsz, uinfo + minsz,
2221 min_t(u32, pasid_table_data.argsz, sizeof(pasid_table_data)) - minsz))
2222 return -EFAULT;
2223
2224 /* Now the argsz is validated, check the content */
> 2225 if (pasid_table_data.config < 1 && pasid_table_data.config > 3)
2226 return -EINVAL;
2227
2228 return domain->ops->attach_pasid_table(domain, &pasid_table_data);
2229 }
2230 EXPORT_SYMBOL_GPL(iommu_uapi_attach_pasid_table);
2231
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39466 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v12 02/15] iommu: Introduce bind/unbind_guest_msi
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
2020-11-16 10:43 ` [PATCH v12 01/15] iommu: Introduce attach/detach_pasid_table API Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 03/15] iommu/arm-smmu-v3: Maintain a SID->device structure Eric Auger
` (12 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
On ARM, MSI are translated by the SMMU. An IOVA is allocated
for each MSI doorbell. If both the host and the guest are exposed
with SMMUs, we end up with 2 different IOVAs allocated by each.
guest allocates an IOVA (gIOVA) to map onto the guest MSI
doorbell (gDB). The Host allocates another IOVA (hIOVA) to map
onto the physical doorbell (hDB).
So we end up with 2 untied mappings:
S1 S2
gIOVA -> gDB
hIOVA -> hDB
Currently the PCI device is programmed by the host with hIOVA
as MSI doorbell. So this does not work.
This patch introduces an API to pass gIOVA/gDB to the host so
that gIOVA can be reused by the host instead of re-allocating
a new IOVA. So the goal is to create the following nested mapping:
S1 S2
gIOVA -> gDB -> hDB
and program the PCI device with gIOVA MSI doorbell.
In case we have several devices attached to this nested domain
(devices belonging to the same group), they cannot be isolated
on guest side either. So they should also end up in the same domain
on guest side. We will enforce that all the devices attached to
the host iommu domain use the same physical doorbell and similarly
a single virtual doorbell mapping gets registered (1 single
virtual doorbell is used on guest as well).
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v7 -> v8:
- dummy iommu_unbind_guest_msi turned into a void function
v6 -> v7:
- remove the device handle parameter.
- Add comments saying there can only be a single MSI binding
registered per iommu_domain
v5 -> v6:
-fix compile issue when IOMMU_API is not set
v3 -> v4:
- add unbind
v2 -> v3:
- add a struct device handle
---
drivers/iommu/iommu.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/iommu.h | 20 ++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b061bf4c3bb2..3f311e25d6e2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2251,6 +2251,43 @@ static void __iommu_detach_device(struct iommu_domain *domain,
trace_detach_device_from_domain(dev);
}
+/**
+ * iommu_bind_guest_msi - Passes the stage1 GIOVA/GPA mapping of a
+ * virtual doorbell
+ *
+ * @domain: iommu domain the stage 1 mapping will be attached to
+ * @iova: iova allocated by the guest
+ * @gpa: guest physical address of the virtual doorbell
+ * @size: granule size used for the mapping
+ *
+ * The associated IOVA can be reused by the host to create a nested
+ * stage2 binding mapping translating into the physical doorbell used
+ * by the devices attached to the domain.
+ *
+ * All devices within the domain must share the same physical doorbell.
+ * A single MSI GIOVA/GPA mapping can be attached to an iommu_domain.
+ */
+
+int iommu_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size)
+{
+ if (unlikely(!domain->ops->bind_guest_msi))
+ return -ENODEV;
+
+ return domain->ops->bind_guest_msi(domain, giova, gpa, size);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_guest_msi);
+
+void iommu_unbind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t iova)
+{
+ if (unlikely(!domain->ops->unbind_guest_msi))
+ return;
+
+ domain->ops->unbind_guest_msi(domain, iova);
+}
+EXPORT_SYMBOL_GPL(iommu_unbind_guest_msi);
+
void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
{
struct iommu_group *group;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 464fcbecf841..35819bff03bc 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -225,6 +225,8 @@ struct iommu_iotlb_gather {
* @sva_unbind_gpasid: unbind guest pasid and mm
* @attach_pasid_table: attach a pasid table
* @detach_pasid_table: detach the pasid table
+ * @bind_guest_msi: provides a stage1 giova/gpa MSI doorbell mapping
+ * @unbind_guest_msi: withdraw a stage1 giova/gpa MSI doorbell mapping
* @def_domain_type: device default domain type, return value:
* - IOMMU_DOMAIN_IDENTITY: must use an identity domain
* - IOMMU_DOMAIN_DMA: must use a dma domain
@@ -305,6 +307,10 @@ struct iommu_ops {
int (*def_domain_type)(struct device *dev);
+ int (*bind_guest_msi)(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size);
+ void (*unbind_guest_msi)(struct iommu_domain *domain, dma_addr_t giova);
+
unsigned long pgsize_bitmap;
struct module *owner;
};
@@ -444,6 +450,10 @@ extern int iommu_attach_pasid_table(struct iommu_domain *domain,
extern int iommu_uapi_attach_pasid_table(struct iommu_domain *domain,
void __user *udata);
extern void iommu_detach_pasid_table(struct iommu_domain *domain);
+extern int iommu_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size);
+extern void iommu_unbind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1087,6 +1097,16 @@ static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
{
return NULL;
}
+
+static inline
+int iommu_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size)
+{
+ return -ENODEV;
+}
+static inline
+void iommu_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova) {}
+
#endif /* CONFIG_IOMMU_API */
/**
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 03/15] iommu/arm-smmu-v3: Maintain a SID->device structure
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
2020-11-16 10:43 ` [PATCH v12 01/15] iommu: Introduce attach/detach_pasid_table API Eric Auger
2020-11-16 10:43 ` [PATCH v12 02/15] iommu: Introduce bind/unbind_guest_msi Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 04/15] iommu/smmuv3: Dynamically allocate s1_cfg and s2_cfg Eric Auger
` (11 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
When handling faults from the event or PRI queue, we need to find the
struct device associated to a SID. Add a rb_tree to keep track of SIDs.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 99 +++++++++++++++++++++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 10 +++
2 files changed, 109 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index e634bbe60573..d828d6cbeb0e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1350,6 +1350,32 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
return 0;
}
+__maybe_unused
+static struct arm_smmu_master *
+arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
+{
+ struct rb_node *node;
+ struct arm_smmu_stream *stream;
+ struct arm_smmu_master *master = NULL;
+
+ mutex_lock(&smmu->streams_mutex);
+ node = smmu->streams.rb_node;
+ while (node) {
+ stream = rb_entry(node, struct arm_smmu_stream, node);
+ if (stream->id < sid) {
+ node = node->rb_right;
+ } else if (stream->id > sid) {
+ node = node->rb_left;
+ } else {
+ master = stream->master;
+ break;
+ }
+ }
+ mutex_unlock(&smmu->streams_mutex);
+
+ return master;
+}
+
/* IRQ and event handlers */
static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
{
@@ -2306,6 +2332,69 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
return sid < limit;
}
+static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
+ struct arm_smmu_master *master)
+{
+ int i;
+ int ret = 0;
+ struct arm_smmu_stream *new_stream, *cur_stream;
+ struct rb_node **new_node, *parent_node = NULL;
+
+ master->streams = kcalloc(master->num_sids,
+ sizeof(struct arm_smmu_stream), GFP_KERNEL);
+ if (!master->streams)
+ return -ENOMEM;
+
+ mutex_lock(&smmu->streams_mutex);
+ for (i = 0; i < master->num_sids && !ret; i++) {
+ new_stream = &master->streams[i];
+ new_stream->id = master->sids[i];
+ new_stream->master = master;
+
+ new_node = &(smmu->streams.rb_node);
+ while (*new_node) {
+ cur_stream = rb_entry(*new_node, struct arm_smmu_stream,
+ node);
+ parent_node = *new_node;
+ if (cur_stream->id > new_stream->id) {
+ new_node = &((*new_node)->rb_left);
+ } else if (cur_stream->id < new_stream->id) {
+ new_node = &((*new_node)->rb_right);
+ } else {
+ dev_warn(master->dev,
+ "stream %u already in tree\n",
+ cur_stream->id);
+ ret = -EINVAL;
+ break;
+ }
+ }
+
+ if (!ret) {
+ rb_link_node(&new_stream->node, parent_node, new_node);
+ rb_insert_color(&new_stream->node, &smmu->streams);
+ }
+ }
+ mutex_unlock(&smmu->streams_mutex);
+
+ return ret;
+}
+
+static void arm_smmu_remove_master(struct arm_smmu_device *smmu,
+ struct arm_smmu_master *master)
+{
+ int i;
+
+ if (!master->streams)
+ return;
+
+ mutex_lock(&smmu->streams_mutex);
+ for (i = 0; i < master->num_sids; i++)
+ rb_erase(&master->streams[i].node, &smmu->streams);
+ mutex_unlock(&smmu->streams_mutex);
+
+ kfree(master->streams);
+}
+
static struct iommu_ops arm_smmu_ops;
static struct iommu_device *arm_smmu_probe_device(struct device *dev)
@@ -2369,6 +2458,10 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
master->ssid_bits = min_t(u8, master->ssid_bits,
CTXDESC_LINEAR_CDMAX);
+ ret = arm_smmu_insert_master(smmu, master);
+ if (ret)
+ goto err_free_master;
+
return &smmu->iommu;
err_free_master:
@@ -2381,14 +2474,17 @@ static void arm_smmu_release_device(struct device *dev)
{
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct arm_smmu_master *master;
+ struct arm_smmu_device *smmu;
if (!fwspec || fwspec->ops != &arm_smmu_ops)
return;
master = dev_iommu_priv_get(dev);
+ smmu = master->smmu;
WARN_ON(arm_smmu_master_sva_enabled(master));
arm_smmu_detach_dev(master);
arm_smmu_disable_pasid(master);
+ arm_smmu_remove_master(smmu, master);
kfree(master);
iommu_fwspec_free(dev);
}
@@ -2808,6 +2904,9 @@ static int arm_smmu_init_structures(struct arm_smmu_device *smmu)
{
int ret;
+ mutex_init(&smmu->streams_mutex);
+ smmu->streams = RB_ROOT;
+
ret = arm_smmu_init_queues(smmu);
if (ret)
return ret;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index d4b7f40ccb02..2944beb1571b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -636,6 +636,15 @@ struct arm_smmu_device {
/* IOMMU core code handle */
struct iommu_device iommu;
+
+ struct rb_root streams;
+ struct mutex streams_mutex;
+};
+
+struct arm_smmu_stream {
+ u32 id;
+ struct arm_smmu_master *master;
+ struct rb_node node;
};
/* SMMU private data for each master */
@@ -646,6 +655,7 @@ struct arm_smmu_master {
struct list_head domain_head;
u32 *sids;
unsigned int num_sids;
+ struct arm_smmu_stream *streams;
bool ats_enabled;
bool sva_enabled;
struct list_head bonds;
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 04/15] iommu/smmuv3: Dynamically allocate s1_cfg and s2_cfg
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (2 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 03/15] iommu/arm-smmu-v3: Maintain a SID->device structure Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-17 11:39 ` Shameerali Kolothum Thodi
2020-11-16 10:43 ` [PATCH v12 05/15] iommu/smmuv3: Get prepared for nested stage support Eric Auger
` (10 subsequent siblings)
14 siblings, 1 reply; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
In preparation for the introduction of nested stages
let's turn s1_cfg and s2_cfg fields into pointers which are
dynamically allocated depending on the smmu_domain stage.
In nested mode, both stages will coexist and s1_cfg will
be allocated when the guest configuration gets passed.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 83 ++++++++++++---------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 6 +-
2 files changed, 48 insertions(+), 41 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index d828d6cbeb0e..4baf9fafe462 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -953,9 +953,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
unsigned int idx;
struct arm_smmu_l1_ctx_desc *l1_desc;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg->cdcfg;
- if (smmu_domain->s1_cfg.s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
+ if (smmu_domain->s1_cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
idx = ssid >> CTXDESC_SPLIT;
@@ -990,7 +990,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
__le64 *cdptr;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax)))
+ if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg->s1cdmax)))
return -E2BIG;
cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
@@ -1056,7 +1056,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
size_t l1size;
size_t max_contexts;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+ struct arm_smmu_s1_cfg *cfg = smmu_domain->s1_cfg;
struct arm_smmu_ctx_desc_cfg *cdcfg = &cfg->cdcfg;
max_contexts = 1 << cfg->s1cdmax;
@@ -1104,7 +1104,7 @@ static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
int i;
size_t size, l1size;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg->cdcfg;
if (cdcfg->l1_desc) {
size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
@@ -1211,17 +1211,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
}
if (smmu_domain) {
- switch (smmu_domain->stage) {
- case ARM_SMMU_DOMAIN_S1:
- s1_cfg = &smmu_domain->s1_cfg;
- break;
- case ARM_SMMU_DOMAIN_S2:
- case ARM_SMMU_DOMAIN_NESTED:
- s2_cfg = &smmu_domain->s2_cfg;
- break;
- default:
- break;
- }
+ s1_cfg = smmu_domain->s1_cfg;
+ s2_cfg = smmu_domain->s2_cfg;
}
if (val & STRTAB_STE_0_V) {
@@ -1664,10 +1655,10 @@ static void arm_smmu_tlb_inv_context(void *cookie)
* careful, 007.
*/
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
- arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg.cd.asid);
+ arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg->cd.asid);
} else {
cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
- cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
+ cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid;
arm_smmu_cmdq_issue_cmd(smmu, &cmd);
arm_smmu_cmdq_issue_sync(smmu);
}
@@ -1693,10 +1684,10 @@ static void arm_smmu_tlb_inv_range(unsigned long iova, size_t size,
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
cmd.opcode = CMDQ_OP_TLBI_NH_VA;
- cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
+ cmd.tlbi.asid = smmu_domain->s1_cfg->cd.asid;
} else {
cmd.opcode = CMDQ_OP_TLBI_S2_IPA;
- cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
+ cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid;
}
if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
@@ -1846,24 +1837,25 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
{
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_s1_cfg *s1_cfg = smmu_domain->s1_cfg;
+ struct arm_smmu_s2_cfg *s2_cfg = smmu_domain->s2_cfg;
iommu_put_dma_cookie(domain);
free_io_pgtable_ops(smmu_domain->pgtbl_ops);
/* Free the CD and ASID, if we allocated them */
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
- struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
-
- /* Prevent SVA from touching the CD while we're freeing it */
+ if (s1_cfg) {
mutex_lock(&arm_smmu_asid_lock);
- if (cfg->cdcfg.cdtab)
+ /* Prevent SVA from touching the CD while we're freeing it */
+ if (s1_cfg->cdcfg.cdtab)
arm_smmu_free_cd_tables(smmu_domain);
- arm_smmu_free_asid(&cfg->cd);
+ arm_smmu_free_asid(&s1_cfg->cd);
mutex_unlock(&arm_smmu_asid_lock);
- } else {
- struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
- if (cfg->vmid)
- arm_smmu_bitmap_free(smmu->vmid_map, cfg->vmid);
+ }
+ if (s2_cfg) {
+ if (s2_cfg->vmid)
+ arm_smmu_bitmap_free(smmu->vmid_map, s2_cfg->vmid);
+ kfree(s2_cfg);
}
kfree(smmu_domain);
@@ -1876,8 +1868,11 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
int ret;
u32 asid;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr = &pgtbl_cfg->arm_lpae_s1_cfg.tcr;
+ struct arm_smmu_s1_cfg *cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
+
+ if (!cfg)
+ return -ENOMEM;
refcount_set(&cfg->cd.refs, 1);
@@ -1890,6 +1885,8 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
cfg->s1cdmax = master->ssid_bits;
+ smmu_domain->s1_cfg = cfg;
+
ret = arm_smmu_alloc_cd_tables(smmu_domain);
if (ret)
goto out_free_asid;
@@ -1922,6 +1919,8 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
out_free_asid:
arm_smmu_free_asid(&cfg->cd);
out_unlock:
+ kfree(cfg);
+ smmu_domain->s1_cfg = NULL;
mutex_unlock(&arm_smmu_asid_lock);
return ret;
}
@@ -1930,14 +1929,19 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
struct arm_smmu_master *master,
struct io_pgtable_cfg *pgtbl_cfg)
{
- int vmid;
+ int vmid, ret;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
+ struct arm_smmu_s2_cfg *cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
typeof(&pgtbl_cfg->arm_lpae_s2_cfg.vtcr) vtcr;
+ if (!cfg)
+ return -ENOMEM;
+
vmid = arm_smmu_bitmap_alloc(smmu->vmid_map, smmu->vmid_bits);
- if (vmid < 0)
- return vmid;
+ if (vmid < 0) {
+ ret = vmid;
+ goto out_free_cfg;
+ }
vtcr = &pgtbl_cfg->arm_lpae_s2_cfg.vtcr;
cfg->vmid = (u16)vmid;
@@ -1949,7 +1953,12 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
FIELD_PREP(STRTAB_STE_2_VTCR_S2SH0, vtcr->sh) |
FIELD_PREP(STRTAB_STE_2_VTCR_S2TG, vtcr->tg) |
FIELD_PREP(STRTAB_STE_2_VTCR_S2PS, vtcr->ps);
+ smmu_domain->s2_cfg = cfg;
return 0;
+
+out_free_cfg:
+ kfree(cfg);
+ return ret;
}
static int arm_smmu_domain_finalise(struct iommu_domain *domain,
@@ -2231,10 +2240,10 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
ret = -ENXIO;
goto out_unlock;
} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
- master->ssid_bits != smmu_domain->s1_cfg.s1cdmax) {
+ master->ssid_bits != smmu_domain->s1_cfg->s1cdmax) {
dev_err(dev,
"cannot attach to incompatible domain (%u SSID bits != %u)\n",
- smmu_domain->s1_cfg.s1cdmax, master->ssid_bits);
+ smmu_domain->s1_cfg->s1cdmax, master->ssid_bits);
ret = -EINVAL;
goto out_unlock;
}
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 2944beb1571b..6fdc35b32dbf 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -679,10 +679,8 @@ struct arm_smmu_domain {
atomic_t nr_ats_masters;
enum arm_smmu_domain_stage stage;
- union {
- struct arm_smmu_s1_cfg s1_cfg;
- struct arm_smmu_s2_cfg s2_cfg;
- };
+ struct arm_smmu_s1_cfg *s1_cfg;
+ struct arm_smmu_s2_cfg *s2_cfg;
struct iommu_domain domain;
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* RE: [PATCH v12 04/15] iommu/smmuv3: Dynamically allocate s1_cfg and s2_cfg
2020-11-16 10:43 ` [PATCH v12 04/15] iommu/smmuv3: Dynamically allocate s1_cfg and s2_cfg Eric Auger
@ 2020-11-17 11:39 ` Shameerali Kolothum Thodi
2020-11-17 12:36 ` Auger Eric
0 siblings, 1 reply; 20+ messages in thread
From: Shameerali Kolothum Thodi @ 2020-11-17 11:39 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
alex.williamson, jacob.jun.pan, yi.l.liu, tn, nicoleotsuka
Hi Eric,
> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@redhat.com]
> Sent: 16 November 2020 10:43
> To: eric.auger.pro@gmail.com; eric.auger@redhat.com;
> iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> kvm@vger.kernel.org; kvmarm@lists.cs.columbia.edu; will@kernel.org;
> joro@8bytes.org; maz@kernel.org; robin.murphy@arm.com
> Cc: jean-philippe@linaro.org; zhangfei.gao@linaro.org;
> zhangfei.gao@gmail.com; vivek.gautam@arm.com; Shameerali Kolothum
> Thodi <shameerali.kolothum.thodi@huawei.com>;
> alex.williamson@redhat.com; jacob.jun.pan@linux.intel.com;
> yi.l.liu@intel.com; tn@semihalf.com; nicoleotsuka@gmail.com
> Subject: [PATCH v12 04/15] iommu/smmuv3: Dynamically allocate s1_cfg and
> s2_cfg
>
> In preparation for the introduction of nested stages
> let's turn s1_cfg and s2_cfg fields into pointers which are
> dynamically allocated depending on the smmu_domain stage.
This will break compile if we have CONFIG_ARM_SMMU_V3_SVA
because ,
https://github.com/eauger/linux/blob/5.10-rc4-2stage-v12/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c#L40
Do we really need to make these pointers?
Thanks,
Shameer
> In nested mode, both stages will coexist and s1_cfg will
> be allocated when the guest configuration gets passed.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 83 ++++++++++++---------
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 6 +-
> 2 files changed, 48 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index d828d6cbeb0e..4baf9fafe462 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -953,9 +953,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct
> arm_smmu_domain *smmu_domain,
> unsigned int idx;
> struct arm_smmu_l1_ctx_desc *l1_desc;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> - struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
> + struct arm_smmu_ctx_desc_cfg *cdcfg =
> &smmu_domain->s1_cfg->cdcfg;
>
> - if (smmu_domain->s1_cfg.s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
> + if (smmu_domain->s1_cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
> return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
>
> idx = ssid >> CTXDESC_SPLIT;
> @@ -990,7 +990,7 @@ int arm_smmu_write_ctx_desc(struct
> arm_smmu_domain *smmu_domain, int ssid,
> __le64 *cdptr;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
>
> - if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax)))
> + if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg->s1cdmax)))
> return -E2BIG;
>
> cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
> @@ -1056,7 +1056,7 @@ static int arm_smmu_alloc_cd_tables(struct
> arm_smmu_domain *smmu_domain)
> size_t l1size;
> size_t max_contexts;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> - struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> + struct arm_smmu_s1_cfg *cfg = smmu_domain->s1_cfg;
> struct arm_smmu_ctx_desc_cfg *cdcfg = &cfg->cdcfg;
>
> max_contexts = 1 << cfg->s1cdmax;
> @@ -1104,7 +1104,7 @@ static void arm_smmu_free_cd_tables(struct
> arm_smmu_domain *smmu_domain)
> int i;
> size_t size, l1size;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> - struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
> + struct arm_smmu_ctx_desc_cfg *cdcfg =
> &smmu_domain->s1_cfg->cdcfg;
>
> if (cdcfg->l1_desc) {
> size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
> @@ -1211,17 +1211,8 @@ static void arm_smmu_write_strtab_ent(struct
> arm_smmu_master *master, u32 sid,
> }
>
> if (smmu_domain) {
> - switch (smmu_domain->stage) {
> - case ARM_SMMU_DOMAIN_S1:
> - s1_cfg = &smmu_domain->s1_cfg;
> - break;
> - case ARM_SMMU_DOMAIN_S2:
> - case ARM_SMMU_DOMAIN_NESTED:
> - s2_cfg = &smmu_domain->s2_cfg;
> - break;
> - default:
> - break;
> - }
> + s1_cfg = smmu_domain->s1_cfg;
> + s2_cfg = smmu_domain->s2_cfg;
> }
>
> if (val & STRTAB_STE_0_V) {
> @@ -1664,10 +1655,10 @@ static void arm_smmu_tlb_inv_context(void
> *cookie)
> * careful, 007.
> */
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> - arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg.cd.asid);
> + arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg->cd.asid);
> } else {
> cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
> - cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
> + cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid;
> arm_smmu_cmdq_issue_cmd(smmu, &cmd);
> arm_smmu_cmdq_issue_sync(smmu);
> }
> @@ -1693,10 +1684,10 @@ static void arm_smmu_tlb_inv_range(unsigned
> long iova, size_t size,
>
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> cmd.opcode = CMDQ_OP_TLBI_NH_VA;
> - cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
> + cmd.tlbi.asid = smmu_domain->s1_cfg->cd.asid;
> } else {
> cmd.opcode = CMDQ_OP_TLBI_S2_IPA;
> - cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
> + cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid;
> }
>
> if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
> @@ -1846,24 +1837,25 @@ static void arm_smmu_domain_free(struct
> iommu_domain *domain)
> {
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_s1_cfg *s1_cfg = smmu_domain->s1_cfg;
> + struct arm_smmu_s2_cfg *s2_cfg = smmu_domain->s2_cfg;
>
> iommu_put_dma_cookie(domain);
> free_io_pgtable_ops(smmu_domain->pgtbl_ops);
>
> /* Free the CD and ASID, if we allocated them */
> - if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> - struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> -
> - /* Prevent SVA from touching the CD while we're freeing it */
> + if (s1_cfg) {
> mutex_lock(&arm_smmu_asid_lock);
> - if (cfg->cdcfg.cdtab)
> + /* Prevent SVA from touching the CD while we're freeing it */
> + if (s1_cfg->cdcfg.cdtab)
> arm_smmu_free_cd_tables(smmu_domain);
> - arm_smmu_free_asid(&cfg->cd);
> + arm_smmu_free_asid(&s1_cfg->cd);
> mutex_unlock(&arm_smmu_asid_lock);
> - } else {
> - struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
> - if (cfg->vmid)
> - arm_smmu_bitmap_free(smmu->vmid_map, cfg->vmid);
> + }
> + if (s2_cfg) {
> + if (s2_cfg->vmid)
> + arm_smmu_bitmap_free(smmu->vmid_map, s2_cfg->vmid);
> + kfree(s2_cfg);
> }
>
> kfree(smmu_domain);
> @@ -1876,8 +1868,11 @@ static int arm_smmu_domain_finalise_s1(struct
> arm_smmu_domain *smmu_domain,
> int ret;
> u32 asid;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> - struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr =
> &pgtbl_cfg->arm_lpae_s1_cfg.tcr;
> + struct arm_smmu_s1_cfg *cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> +
> + if (!cfg)
> + return -ENOMEM;
>
> refcount_set(&cfg->cd.refs, 1);
>
> @@ -1890,6 +1885,8 @@ static int arm_smmu_domain_finalise_s1(struct
> arm_smmu_domain *smmu_domain,
>
> cfg->s1cdmax = master->ssid_bits;
>
> + smmu_domain->s1_cfg = cfg;
> +
> ret = arm_smmu_alloc_cd_tables(smmu_domain);
> if (ret)
> goto out_free_asid;
> @@ -1922,6 +1919,8 @@ static int arm_smmu_domain_finalise_s1(struct
> arm_smmu_domain *smmu_domain,
> out_free_asid:
> arm_smmu_free_asid(&cfg->cd);
> out_unlock:
> + kfree(cfg);
> + smmu_domain->s1_cfg = NULL;
> mutex_unlock(&arm_smmu_asid_lock);
> return ret;
> }
> @@ -1930,14 +1929,19 @@ static int arm_smmu_domain_finalise_s2(struct
> arm_smmu_domain *smmu_domain,
> struct arm_smmu_master *master,
> struct io_pgtable_cfg *pgtbl_cfg)
> {
> - int vmid;
> + int vmid, ret;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> - struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
> + struct arm_smmu_s2_cfg *cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> typeof(&pgtbl_cfg->arm_lpae_s2_cfg.vtcr) vtcr;
>
> + if (!cfg)
> + return -ENOMEM;
> +
> vmid = arm_smmu_bitmap_alloc(smmu->vmid_map, smmu->vmid_bits);
> - if (vmid < 0)
> - return vmid;
> + if (vmid < 0) {
> + ret = vmid;
> + goto out_free_cfg;
> + }
>
> vtcr = &pgtbl_cfg->arm_lpae_s2_cfg.vtcr;
> cfg->vmid = (u16)vmid;
> @@ -1949,7 +1953,12 @@ static int arm_smmu_domain_finalise_s2(struct
> arm_smmu_domain *smmu_domain,
> FIELD_PREP(STRTAB_STE_2_VTCR_S2SH0, vtcr->sh) |
> FIELD_PREP(STRTAB_STE_2_VTCR_S2TG, vtcr->tg) |
> FIELD_PREP(STRTAB_STE_2_VTCR_S2PS, vtcr->ps);
> + smmu_domain->s2_cfg = cfg;
> return 0;
> +
> +out_free_cfg:
> + kfree(cfg);
> + return ret;
> }
>
> static int arm_smmu_domain_finalise(struct iommu_domain *domain,
> @@ -2231,10 +2240,10 @@ static int arm_smmu_attach_dev(struct
> iommu_domain *domain, struct device *dev)
> ret = -ENXIO;
> goto out_unlock;
> } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
> - master->ssid_bits != smmu_domain->s1_cfg.s1cdmax) {
> + master->ssid_bits != smmu_domain->s1_cfg->s1cdmax) {
> dev_err(dev,
> "cannot attach to incompatible domain (%u SSID bits != %u)\n",
> - smmu_domain->s1_cfg.s1cdmax, master->ssid_bits);
> + smmu_domain->s1_cfg->s1cdmax, master->ssid_bits);
> ret = -EINVAL;
> goto out_unlock;
> }
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 2944beb1571b..6fdc35b32dbf 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -679,10 +679,8 @@ struct arm_smmu_domain {
> atomic_t nr_ats_masters;
>
> enum arm_smmu_domain_stage stage;
> - union {
> - struct arm_smmu_s1_cfg s1_cfg;
> - struct arm_smmu_s2_cfg s2_cfg;
> - };
> + struct arm_smmu_s1_cfg *s1_cfg;
> + struct arm_smmu_s2_cfg *s2_cfg;
>
> struct iommu_domain domain;
>
> --
> 2.21.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v12 04/15] iommu/smmuv3: Dynamically allocate s1_cfg and s2_cfg
2020-11-17 11:39 ` Shameerali Kolothum Thodi
@ 2020-11-17 12:36 ` Auger Eric
0 siblings, 0 replies; 20+ messages in thread
From: Auger Eric @ 2020-11-17 12:36 UTC (permalink / raw)
To: Shameerali Kolothum Thodi, eric.auger.pro, iommu, linux-kernel,
kvm, kvmarm, will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
alex.williamson, jacob.jun.pan, yi.l.liu, tn, nicoleotsuka
Hi Shameer,
On 11/17/20 12:39 PM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger [mailto:eric.auger@redhat.com]
>> Sent: 16 November 2020 10:43
>> To: eric.auger.pro@gmail.com; eric.auger@redhat.com;
>> iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
>> kvm@vger.kernel.org; kvmarm@lists.cs.columbia.edu; will@kernel.org;
>> joro@8bytes.org; maz@kernel.org; robin.murphy@arm.com
>> Cc: jean-philippe@linaro.org; zhangfei.gao@linaro.org;
>> zhangfei.gao@gmail.com; vivek.gautam@arm.com; Shameerali Kolothum
>> Thodi <shameerali.kolothum.thodi@huawei.com>;
>> alex.williamson@redhat.com; jacob.jun.pan@linux.intel.com;
>> yi.l.liu@intel.com; tn@semihalf.com; nicoleotsuka@gmail.com
>> Subject: [PATCH v12 04/15] iommu/smmuv3: Dynamically allocate s1_cfg and
>> s2_cfg
>>
>> In preparation for the introduction of nested stages
>> let's turn s1_cfg and s2_cfg fields into pointers which are
>> dynamically allocated depending on the smmu_domain stage.
>
> This will break compile if we have CONFIG_ARM_SMMU_V3_SVA
> because ,
> https://github.com/eauger/linux/blob/5.10-rc4-2stage-v12/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c#L40
>
> Do we really need to make these pointers?
Thanks for reporting. I think I can do differently. Working on this now.
Thanks
Eric
>
> Thanks,
> Shameer
>
>> In nested mode, both stages will coexist and s1_cfg will
>> be allocated when the guest configuration gets passed.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 83 ++++++++++++---------
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 6 +-
>> 2 files changed, 48 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> index d828d6cbeb0e..4baf9fafe462 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> @@ -953,9 +953,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct
>> arm_smmu_domain *smmu_domain,
>> unsigned int idx;
>> struct arm_smmu_l1_ctx_desc *l1_desc;
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>> - struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
>> + struct arm_smmu_ctx_desc_cfg *cdcfg =
>> &smmu_domain->s1_cfg->cdcfg;
>>
>> - if (smmu_domain->s1_cfg.s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
>> + if (smmu_domain->s1_cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
>> return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
>>
>> idx = ssid >> CTXDESC_SPLIT;
>> @@ -990,7 +990,7 @@ int arm_smmu_write_ctx_desc(struct
>> arm_smmu_domain *smmu_domain, int ssid,
>> __le64 *cdptr;
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>>
>> - if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax)))
>> + if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg->s1cdmax)))
>> return -E2BIG;
>>
>> cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
>> @@ -1056,7 +1056,7 @@ static int arm_smmu_alloc_cd_tables(struct
>> arm_smmu_domain *smmu_domain)
>> size_t l1size;
>> size_t max_contexts;
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>> - struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>> + struct arm_smmu_s1_cfg *cfg = smmu_domain->s1_cfg;
>> struct arm_smmu_ctx_desc_cfg *cdcfg = &cfg->cdcfg;
>>
>> max_contexts = 1 << cfg->s1cdmax;
>> @@ -1104,7 +1104,7 @@ static void arm_smmu_free_cd_tables(struct
>> arm_smmu_domain *smmu_domain)
>> int i;
>> size_t size, l1size;
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>> - struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
>> + struct arm_smmu_ctx_desc_cfg *cdcfg =
>> &smmu_domain->s1_cfg->cdcfg;
>>
>> if (cdcfg->l1_desc) {
>> size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
>> @@ -1211,17 +1211,8 @@ static void arm_smmu_write_strtab_ent(struct
>> arm_smmu_master *master, u32 sid,
>> }
>>
>> if (smmu_domain) {
>> - switch (smmu_domain->stage) {
>> - case ARM_SMMU_DOMAIN_S1:
>> - s1_cfg = &smmu_domain->s1_cfg;
>> - break;
>> - case ARM_SMMU_DOMAIN_S2:
>> - case ARM_SMMU_DOMAIN_NESTED:
>> - s2_cfg = &smmu_domain->s2_cfg;
>> - break;
>> - default:
>> - break;
>> - }
>> + s1_cfg = smmu_domain->s1_cfg;
>> + s2_cfg = smmu_domain->s2_cfg;
>> }
>>
>> if (val & STRTAB_STE_0_V) {
>> @@ -1664,10 +1655,10 @@ static void arm_smmu_tlb_inv_context(void
>> *cookie)
>> * careful, 007.
>> */
>> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
>> - arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg.cd.asid);
>> + arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg->cd.asid);
>> } else {
>> cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
>> - cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
>> + cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid;
>> arm_smmu_cmdq_issue_cmd(smmu, &cmd);
>> arm_smmu_cmdq_issue_sync(smmu);
>> }
>> @@ -1693,10 +1684,10 @@ static void arm_smmu_tlb_inv_range(unsigned
>> long iova, size_t size,
>>
>> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
>> cmd.opcode = CMDQ_OP_TLBI_NH_VA;
>> - cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
>> + cmd.tlbi.asid = smmu_domain->s1_cfg->cd.asid;
>> } else {
>> cmd.opcode = CMDQ_OP_TLBI_S2_IPA;
>> - cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
>> + cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid;
>> }
>>
>> if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
>> @@ -1846,24 +1837,25 @@ static void arm_smmu_domain_free(struct
>> iommu_domain *domain)
>> {
>> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>> + struct arm_smmu_s1_cfg *s1_cfg = smmu_domain->s1_cfg;
>> + struct arm_smmu_s2_cfg *s2_cfg = smmu_domain->s2_cfg;
>>
>> iommu_put_dma_cookie(domain);
>> free_io_pgtable_ops(smmu_domain->pgtbl_ops);
>>
>> /* Free the CD and ASID, if we allocated them */
>> - if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
>> - struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>> -
>> - /* Prevent SVA from touching the CD while we're freeing it */
>> + if (s1_cfg) {
>> mutex_lock(&arm_smmu_asid_lock);
>> - if (cfg->cdcfg.cdtab)
>> + /* Prevent SVA from touching the CD while we're freeing it */
>> + if (s1_cfg->cdcfg.cdtab)
>> arm_smmu_free_cd_tables(smmu_domain);
>> - arm_smmu_free_asid(&cfg->cd);
>> + arm_smmu_free_asid(&s1_cfg->cd);
>> mutex_unlock(&arm_smmu_asid_lock);
>> - } else {
>> - struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
>> - if (cfg->vmid)
>> - arm_smmu_bitmap_free(smmu->vmid_map, cfg->vmid);
>> + }
>> + if (s2_cfg) {
>> + if (s2_cfg->vmid)
>> + arm_smmu_bitmap_free(smmu->vmid_map, s2_cfg->vmid);
>> + kfree(s2_cfg);
>> }
>>
>> kfree(smmu_domain);
>> @@ -1876,8 +1868,11 @@ static int arm_smmu_domain_finalise_s1(struct
>> arm_smmu_domain *smmu_domain,
>> int ret;
>> u32 asid;
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>> - struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>> typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr =
>> &pgtbl_cfg->arm_lpae_s1_cfg.tcr;
>> + struct arm_smmu_s1_cfg *cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
>> +
>> + if (!cfg)
>> + return -ENOMEM;
>>
>> refcount_set(&cfg->cd.refs, 1);
>>
>> @@ -1890,6 +1885,8 @@ static int arm_smmu_domain_finalise_s1(struct
>> arm_smmu_domain *smmu_domain,
>>
>> cfg->s1cdmax = master->ssid_bits;
>>
>> + smmu_domain->s1_cfg = cfg;
>> +
>> ret = arm_smmu_alloc_cd_tables(smmu_domain);
>> if (ret)
>> goto out_free_asid;
>> @@ -1922,6 +1919,8 @@ static int arm_smmu_domain_finalise_s1(struct
>> arm_smmu_domain *smmu_domain,
>> out_free_asid:
>> arm_smmu_free_asid(&cfg->cd);
>> out_unlock:
>> + kfree(cfg);
>> + smmu_domain->s1_cfg = NULL;
>> mutex_unlock(&arm_smmu_asid_lock);
>> return ret;
>> }
>> @@ -1930,14 +1929,19 @@ static int arm_smmu_domain_finalise_s2(struct
>> arm_smmu_domain *smmu_domain,
>> struct arm_smmu_master *master,
>> struct io_pgtable_cfg *pgtbl_cfg)
>> {
>> - int vmid;
>> + int vmid, ret;
>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>> - struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
>> + struct arm_smmu_s2_cfg *cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
>> typeof(&pgtbl_cfg->arm_lpae_s2_cfg.vtcr) vtcr;
>>
>> + if (!cfg)
>> + return -ENOMEM;
>> +
>> vmid = arm_smmu_bitmap_alloc(smmu->vmid_map, smmu->vmid_bits);
>> - if (vmid < 0)
>> - return vmid;
>> + if (vmid < 0) {
>> + ret = vmid;
>> + goto out_free_cfg;
>> + }
>>
>> vtcr = &pgtbl_cfg->arm_lpae_s2_cfg.vtcr;
>> cfg->vmid = (u16)vmid;
>> @@ -1949,7 +1953,12 @@ static int arm_smmu_domain_finalise_s2(struct
>> arm_smmu_domain *smmu_domain,
>> FIELD_PREP(STRTAB_STE_2_VTCR_S2SH0, vtcr->sh) |
>> FIELD_PREP(STRTAB_STE_2_VTCR_S2TG, vtcr->tg) |
>> FIELD_PREP(STRTAB_STE_2_VTCR_S2PS, vtcr->ps);
>> + smmu_domain->s2_cfg = cfg;
>> return 0;
>> +
>> +out_free_cfg:
>> + kfree(cfg);
>> + return ret;
>> }
>>
>> static int arm_smmu_domain_finalise(struct iommu_domain *domain,
>> @@ -2231,10 +2240,10 @@ static int arm_smmu_attach_dev(struct
>> iommu_domain *domain, struct device *dev)
>> ret = -ENXIO;
>> goto out_unlock;
>> } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
>> - master->ssid_bits != smmu_domain->s1_cfg.s1cdmax) {
>> + master->ssid_bits != smmu_domain->s1_cfg->s1cdmax) {
>> dev_err(dev,
>> "cannot attach to incompatible domain (%u SSID bits != %u)\n",
>> - smmu_domain->s1_cfg.s1cdmax, master->ssid_bits);
>> + smmu_domain->s1_cfg->s1cdmax, master->ssid_bits);
>> ret = -EINVAL;
>> goto out_unlock;
>> }
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
>> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
>> index 2944beb1571b..6fdc35b32dbf 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
>> @@ -679,10 +679,8 @@ struct arm_smmu_domain {
>> atomic_t nr_ats_masters;
>>
>> enum arm_smmu_domain_stage stage;
>> - union {
>> - struct arm_smmu_s1_cfg s1_cfg;
>> - struct arm_smmu_s2_cfg s2_cfg;
>> - };
>> + struct arm_smmu_s1_cfg *s1_cfg;
>> + struct arm_smmu_s2_cfg *s2_cfg;
>>
>> struct iommu_domain domain;
>>
>> --
>> 2.21.3
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v12 05/15] iommu/smmuv3: Get prepared for nested stage support
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (3 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 04/15] iommu/smmuv3: Dynamically allocate s1_cfg and s2_cfg Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 06/15] iommu/smmuv3: Implement attach/detach_pasid_table Eric Auger
` (9 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
When nested stage translation is setup, both s1_cfg and
s2_cfg are allocated.
We introduce a new smmu domain abort field that will be set
upon guest stage1 configuration passing.
arm_smmu_write_strtab_ent() is modified to write both stage
fields in the STE and deal with the abort field.
In nested mode, only stage 2 is "finalized" as the host does
not own/configure the stage 1 context descriptor; guest does.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v10 -> v11:
- Fix an issue reported by Shameer when switching from with vSMMU
to without vSMMU. Despite the spec does not seem to mention it
seems to be needed to reset the 2 high 64b when switching from
S1+S2 cfg to S1 only. Especially dst[3] needs to be reset (S2TTB).
On some implementations, if the S2TTB is not reset, this causes
a C_BAD_STE error
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 66 +++++++++++++++++----
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +
2 files changed, 58 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 4baf9fafe462..9580090bd0c9 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1181,8 +1181,10 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
* three cases at the moment:
*
* 1. Invalid (all zero) -> bypass/fault (init)
- * 2. Bypass/fault -> translation/bypass (attach)
- * 3. Translation/bypass -> bypass/fault (detach)
+ * 2. Bypass/fault -> single stage translation/bypass (attach)
+ * 3. Single or nested stage Translation/bypass -> bypass/fault (detach)
+ * 4. S2 -> S1 + S2 (attach_pasid_table)
+ * 5. S1 + S2 -> S2 (detach_pasid_table)
*
* Given that we can't update the STE atomically and the SMMU
* doesn't read the thing in a defined order, that leaves us
@@ -1193,7 +1195,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
* 3. Update Config, sync
*/
u64 val = le64_to_cpu(dst[0]);
- bool ste_live = false;
+ bool abort, translate, s1_live = false, s2_live = false, ste_live;
+ bool nested = false;
struct arm_smmu_device *smmu = NULL;
struct arm_smmu_s1_cfg *s1_cfg = NULL;
struct arm_smmu_s2_cfg *s2_cfg = NULL;
@@ -1213,6 +1216,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
if (smmu_domain) {
s1_cfg = smmu_domain->s1_cfg;
s2_cfg = smmu_domain->s2_cfg;
+ nested = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
}
if (val & STRTAB_STE_0_V) {
@@ -1220,23 +1224,37 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
case STRTAB_STE_0_CFG_BYPASS:
break;
case STRTAB_STE_0_CFG_S1_TRANS:
+ s1_live = true;
+ break;
case STRTAB_STE_0_CFG_S2_TRANS:
- ste_live = true;
+ s2_live = true;
+ break;
+ case STRTAB_STE_0_CFG_NESTED:
+ s1_live = true;
+ s2_live = true;
break;
case STRTAB_STE_0_CFG_ABORT:
- BUG_ON(!disable_bypass);
break;
default:
BUG(); /* STE corruption */
}
}
+ ste_live = s1_live || s2_live;
+
/* Nuke the existing STE_0 value, as we're going to rewrite it */
val = STRTAB_STE_0_V;
/* Bypass/fault */
- if (!smmu_domain || !(s1_cfg || s2_cfg)) {
- if (!smmu_domain && disable_bypass)
+
+ if (!smmu_domain)
+ abort = disable_bypass;
+ else
+ abort = smmu_domain->abort;
+ translate = s1_cfg || s2_cfg;
+
+ if (abort || !translate) {
+ if (abort)
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
else
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
@@ -1254,8 +1272,18 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
return;
}
+ /* S1 or S2 translation */
+
+ BUG_ON(ste_live && !nested);
+
+ if (ste_live) {
+ /* First invalidate the live STE */
+ dst[0] = cpu_to_le64(STRTAB_STE_0_CFG_ABORT);
+ arm_smmu_sync_ste_for_sid(smmu, sid);
+ }
+
if (s1_cfg) {
- BUG_ON(ste_live);
+ BUG_ON(s1_live);
dst[1] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
@@ -1274,7 +1302,14 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
}
if (s2_cfg) {
- BUG_ON(ste_live);
+ u64 vttbr = s2_cfg->vttbr & STRTAB_STE_3_S2TTB_MASK;
+
+ if (s2_live) {
+ u64 s2ttb = le64_to_cpu(dst[3] & STRTAB_STE_3_S2TTB_MASK);
+
+ BUG_ON(s2ttb != vttbr);
+ }
+
dst[2] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_2_S2VMID, s2_cfg->vmid) |
FIELD_PREP(STRTAB_STE_2_VTCR, s2_cfg->vtcr) |
@@ -1284,9 +1319,12 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
STRTAB_STE_2_S2PTW | STRTAB_STE_2_S2AA64 |
STRTAB_STE_2_S2R);
- dst[3] = cpu_to_le64(s2_cfg->vttbr & STRTAB_STE_3_S2TTB_MASK);
+ dst[3] = cpu_to_le64(vttbr);
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S2_TRANS);
+ } else {
+ dst[2] = 0;
+ dst[3] = 0;
}
if (master->ats_enabled)
@@ -1980,6 +2018,14 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
return 0;
}
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED &&
+ (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1) ||
+ !(smmu->features & ARM_SMMU_FEAT_TRANS_S2))) {
+ dev_info(smmu_domain->smmu->dev,
+ "does not implement two stages\n");
+ return -EINVAL;
+ }
+
/* Restrict the stage to what we can actually support */
if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1))
smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 6fdc35b32dbf..f30d0384f906 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -206,6 +206,7 @@
#define STRTAB_STE_0_CFG_BYPASS 4
#define STRTAB_STE_0_CFG_S1_TRANS 5
#define STRTAB_STE_0_CFG_S2_TRANS 6
+#define STRTAB_STE_0_CFG_NESTED 7
#define STRTAB_STE_0_S1FMT GENMASK_ULL(5, 4)
#define STRTAB_STE_0_S1FMT_LINEAR 0
@@ -681,6 +682,7 @@ struct arm_smmu_domain {
enum arm_smmu_domain_stage stage;
struct arm_smmu_s1_cfg *s1_cfg;
struct arm_smmu_s2_cfg *s2_cfg;
+ bool abort;
struct iommu_domain domain;
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 06/15] iommu/smmuv3: Implement attach/detach_pasid_table
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (4 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 05/15] iommu/smmuv3: Get prepared for nested stage support Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 07/15] iommu/smmuv3: Allow stage 1 invalidation with unmanaged ASIDs Eric Auger
` (8 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
On attach_pasid_table() we program STE S1 related info set
by the guest into the actual physical STEs. At minimum
we need to program the context descriptor GPA and compute
whether the stage1 is translated/bypassed or aborted.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v7 -> v8:
- remove smmu->features check, now done on domain finalize
v6 -> v7:
- check versions and comment the fact we don't need to take
into account s1dss and s1fmt
v3 -> v4:
- adapt to changes in iommu_pasid_table_config
- different programming convention at s1_cfg/s2_cfg/ste.abort
v2 -> v3:
- callback now is named set_pasid_table and struct fields
are laid out differently.
v1 -> v2:
- invalidate the STE before changing them
- hold init_mutex
- handle new fields
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 98 +++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 9580090bd0c9..08ab0dd81049 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2655,6 +2655,102 @@ static void arm_smmu_get_resv_regions(struct device *dev,
iommu_dma_get_resv_regions(dev, head);
}
+static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_master *master;
+ struct arm_smmu_device *smmu;
+ unsigned long flags;
+ int ret = -EINVAL;
+
+ if (cfg->format != IOMMU_PASID_FORMAT_SMMUV3)
+ return -EINVAL;
+
+ if (cfg->version != PASID_TABLE_CFG_VERSION_1 ||
+ cfg->vendor_data.smmuv3.version != PASID_TABLE_SMMUV3_CFG_VERSION_1)
+ return -EINVAL;
+
+ mutex_lock(&smmu_domain->init_mutex);
+
+ smmu = smmu_domain->smmu;
+
+ if (!smmu)
+ goto out;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto out;
+
+ switch (cfg->config) {
+ case IOMMU_PASID_CONFIG_ABORT:
+ kfree(smmu_domain->s1_cfg);
+ smmu_domain->s1_cfg = NULL;
+ smmu_domain->abort = true;
+ break;
+ case IOMMU_PASID_CONFIG_BYPASS:
+ kfree(smmu_domain->s1_cfg);
+ smmu_domain->s1_cfg = NULL;
+ smmu_domain->abort = false;
+ break;
+ case IOMMU_PASID_CONFIG_TRANSLATE:
+ /* we do not support S1 <-> S1 transitions */
+ if (smmu_domain->s1_cfg)
+ goto out;
+
+ /*
+ * we currently support a single CD so s1fmt and s1dss
+ * fields are also ignored
+ */
+ if (cfg->pasid_bits)
+ goto out;
+
+ smmu_domain->s1_cfg = kzalloc(sizeof(*smmu_domain->s1_cfg),
+ GFP_KERNEL);
+ if (!smmu_domain->s1_cfg) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ smmu_domain->s1_cfg->cdcfg.cdtab_dma = cfg->base_ptr;
+ smmu_domain->abort = false;
+ break;
+ default:
+ goto out;
+ }
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, domain_head)
+ arm_smmu_install_ste_for_dev(master);
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+ ret = 0;
+out:
+ mutex_unlock(&smmu_domain->init_mutex);
+ return ret;
+}
+
+static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_master *master;
+ unsigned long flags;
+
+ mutex_lock(&smmu_domain->init_mutex);
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto unlock;
+
+ kfree(smmu_domain->s1_cfg);
+ smmu_domain->s1_cfg = NULL;
+ smmu_domain->abort = true;
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, domain_head)
+ arm_smmu_install_ste_for_dev(master);
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+unlock:
+ mutex_unlock(&smmu_domain->init_mutex);
+}
+
static bool arm_smmu_dev_has_feature(struct device *dev,
enum iommu_dev_features feat)
{
@@ -2736,6 +2832,8 @@ static struct iommu_ops arm_smmu_ops = {
.of_xlate = arm_smmu_of_xlate,
.get_resv_regions = arm_smmu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
+ .attach_pasid_table = arm_smmu_attach_pasid_table,
+ .detach_pasid_table = arm_smmu_detach_pasid_table,
.dev_has_feat = arm_smmu_dev_has_feature,
.dev_feat_enabled = arm_smmu_dev_feature_enabled,
.dev_enable_feat = arm_smmu_dev_enable_feature,
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 07/15] iommu/smmuv3: Allow stage 1 invalidation with unmanaged ASIDs
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (5 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 06/15] iommu/smmuv3: Implement attach/detach_pasid_table Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 08/15] iommu/smmuv3: Implement cache_invalidate Eric Auger
` (7 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
With nested stage support, soon we will need to invalidate
S1 contexts and ranges tagged with an unmanaged asid, this
latter being managed by the guest. So let's introduce 2 helpers
that allow to invalidate with externally managed ASIDs
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 35 +++++++++++++++++----
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 08ab0dd81049..73f7a56101dd 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1679,9 +1679,9 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
}
/* IO_PGTABLE API */
-static void arm_smmu_tlb_inv_context(void *cookie)
+static void __arm_smmu_tlb_inv_context(struct arm_smmu_domain *smmu_domain,
+ int ext_asid)
{
- struct arm_smmu_domain *smmu_domain = cookie;
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cmdq_ent cmd;
@@ -1692,7 +1692,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)
* insertion to guarantee those are observed before the TLBI. Do be
* careful, 007.
*/
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+ if (ext_asid >= 0) { /* guest stage 1 invalidation */
+ cmd.opcode = CMDQ_OP_TLBI_NH_ASID;
+ cmd.tlbi.asid = ext_asid;
+ cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid;
+ } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg->cd.asid);
} else {
cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
@@ -1703,9 +1707,17 @@ static void arm_smmu_tlb_inv_context(void *cookie)
arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
}
-static void arm_smmu_tlb_inv_range(unsigned long iova, size_t size,
+static void arm_smmu_tlb_inv_context(void *cookie)
+{
+ struct arm_smmu_domain *smmu_domain = cookie;
+
+ __arm_smmu_tlb_inv_context(smmu_domain, -1);
+}
+
+static void __arm_smmu_tlb_inv_range(unsigned long iova, size_t size,
size_t granule, bool leaf,
- struct arm_smmu_domain *smmu_domain)
+ struct arm_smmu_domain *smmu_domain,
+ int ext_asid)
{
struct arm_smmu_device *smmu = smmu_domain->smmu;
unsigned long start = iova, end = iova + size, num_pages = 0, tg = 0;
@@ -1720,7 +1732,11 @@ static void arm_smmu_tlb_inv_range(unsigned long iova, size_t size,
if (!size)
return;
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+ if (ext_asid >= 0) { /* guest stage 1 invalidation */
+ cmd.opcode = CMDQ_OP_TLBI_NH_VA;
+ cmd.tlbi.asid = ext_asid;
+ cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid;
+ } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
cmd.opcode = CMDQ_OP_TLBI_NH_VA;
cmd.tlbi.asid = smmu_domain->s1_cfg->cd.asid;
} else {
@@ -1780,6 +1796,13 @@ static void arm_smmu_tlb_inv_range(unsigned long iova, size_t size,
arm_smmu_atc_inv_domain(smmu_domain, 0, start, size);
}
+static void arm_smmu_tlb_inv_range(unsigned long iova, size_t size,
+ size_t granule, bool leaf,
+ struct arm_smmu_domain *smmu_domain)
+{
+ __arm_smmu_tlb_inv_range(iova, size, granule, leaf, smmu_domain, -1);
+}
+
static void arm_smmu_tlb_inv_page_nosync(struct iommu_iotlb_gather *gather,
unsigned long iova, size_t granule,
void *cookie)
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 08/15] iommu/smmuv3: Implement cache_invalidate
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (6 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 07/15] iommu/smmuv3: Allow stage 1 invalidation with unmanaged ASIDs Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 09/15] dma-iommu: Implement NESTED_MSI cookie Eric Auger
` (6 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
Implement domain-selective and page-selective IOTLB invalidations.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v7 -> v8:
- ASID based invalidation using iommu_inv_pasid_info
- check ARCHID/PASID flags in addr based invalidation
- use __arm_smmu_tlb_inv_context and __arm_smmu_tlb_inv_range_nosync
v6 -> v7
- check the uapi version
v3 -> v4:
- adapt to changes in the uapi
- add support for leaf parameter
- do not use arm_smmu_tlb_inv_range_nosync or arm_smmu_tlb_inv_context
anymore
v2 -> v3:
- replace __arm_smmu_tlb_sync by arm_smmu_cmdq_issue_sync
v1 -> v2:
- properly pass the asid
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 53 +++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 73f7a56101dd..4b796693d697 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2774,6 +2774,58 @@ static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
mutex_unlock(&smmu_domain->init_mutex);
}
+static int
+arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ return -EINVAL;
+
+ if (!smmu)
+ return -EINVAL;
+
+ if (inv_info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
+ return -EINVAL;
+
+ if (inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB) {
+ if (inv_info->granularity == IOMMU_INV_GRANU_PASID) {
+ struct iommu_inv_pasid_info *info =
+ &inv_info->granu.pasid_info;
+
+ if (!(info->flags & IOMMU_INV_PASID_FLAGS_ARCHID) ||
+ (info->flags & IOMMU_INV_PASID_FLAGS_PASID))
+ return -EINVAL;
+
+ __arm_smmu_tlb_inv_context(smmu_domain, info->archid);
+
+ } else if (inv_info->granularity == IOMMU_INV_GRANU_ADDR) {
+ struct iommu_inv_addr_info *info = &inv_info->granu.addr_info;
+ size_t size = info->nb_granules * info->granule_size;
+ bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF;
+
+ if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID) ||
+ (info->flags & IOMMU_INV_ADDR_FLAGS_PASID))
+ return -EINVAL;
+
+ __arm_smmu_tlb_inv_range(info->addr, size,
+ info->granule_size, leaf,
+ smmu_domain, info->archid);
+
+ arm_smmu_cmdq_issue_sync(smmu);
+ } else {
+ return -EINVAL;
+ }
+ }
+ if (inv_info->cache & IOMMU_CACHE_INV_TYPE_PASID ||
+ inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) {
+ return -ENOENT;
+ }
+ return 0;
+}
+
static bool arm_smmu_dev_has_feature(struct device *dev,
enum iommu_dev_features feat)
{
@@ -2857,6 +2909,7 @@ static struct iommu_ops arm_smmu_ops = {
.put_resv_regions = generic_iommu_put_resv_regions,
.attach_pasid_table = arm_smmu_attach_pasid_table,
.detach_pasid_table = arm_smmu_detach_pasid_table,
+ .cache_invalidate = arm_smmu_cache_invalidate,
.dev_has_feat = arm_smmu_dev_has_feature,
.dev_feat_enabled = arm_smmu_dev_feature_enabled,
.dev_enable_feat = arm_smmu_dev_enable_feature,
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 09/15] dma-iommu: Implement NESTED_MSI cookie
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (7 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 08/15] iommu/smmuv3: Implement cache_invalidate Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 10/15] iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement Eric Auger
` (5 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
Up to now, when the type was UNMANAGED, we used to
allocate IOVA pages within a reserved IOVA MSI range.
If both the host and the guest are exposed with SMMUs, each
would allocate an IOVA. The guest allocates an IOVA (gIOVA)
to map onto the guest MSI doorbell (gDB). The Host allocates
another IOVA (hIOVA) to map onto the physical doorbell (hDB).
So we end up with 2 unrelated mappings, at S1 and S2:
S1 S2
gIOVA -> gDB
hIOVA -> hDB
The PCI device would be programmed with hIOVA.
No stage 1 mapping would existing, causing the MSIs to fault.
iommu_dma_bind_guest_msi() allows to pass gIOVA/gDB
to the host so that gIOVA can be used by the host instead of
re-allocating a new hIOVA.
S1 S2
gIOVA -> gDB -> hDB
this time, the PCI device can be programmed with the gIOVA MSI
doorbell which is correctly mapped through both stages.
Nested mode is not compatible with HW MSI regions as in that
case gDB and hDB should have a 1-1 mapping. This check will
be done when attaching each device to the IOMMU domain.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v10 -> v11:
- fix compilation if !CONFIG_IOMMU_DMA
v7 -> v8:
- correct iommu_dma_(un)bind_guest_msi when
!CONFIG_IOMMU_DMA
- Mentioned nested mode is not compatible with HW MSI regions
in commit message
- protect with msi_lock on unbind
v6 -> v7:
- removed device handle
v3 -> v4:
- change function names; add unregister
- protect with msi_lock
v2 -> v3:
- also store the device handle on S1 mapping registration.
This garantees we associate the associated S2 mapping binds
to the correct physical MSI controller.
v1 -> v2:
- unmap stage2 on put()
---
drivers/iommu/dma-iommu.c | 142 +++++++++++++++++++++++++++++++++++++-
include/linux/dma-iommu.h | 16 +++++
2 files changed, 155 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 0cbcd3fc3e7e..a14ecad6b79b 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -19,6 +19,7 @@
#include <linux/irq.h>
#include <linux/mm.h>
#include <linux/mutex.h>
+#include <linux/mutex.h>
#include <linux/pci.h>
#include <linux/scatterlist.h>
#include <linux/vmalloc.h>
@@ -27,12 +28,15 @@
struct iommu_dma_msi_page {
struct list_head list;
dma_addr_t iova;
+ dma_addr_t gpa;
phys_addr_t phys;
+ size_t s1_granule;
};
enum iommu_dma_cookie_type {
IOMMU_DMA_IOVA_COOKIE,
IOMMU_DMA_MSI_COOKIE,
+ IOMMU_DMA_NESTED_MSI_COOKIE,
};
struct iommu_dma_cookie {
@@ -44,6 +48,7 @@ struct iommu_dma_cookie {
dma_addr_t msi_iova;
};
struct list_head msi_page_list;
+ spinlock_t msi_lock;
/* Domain for flush queue callback; NULL if flush queue not in use */
struct iommu_domain *fq_domain;
@@ -62,6 +67,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
if (cookie) {
+ spin_lock_init(&cookie->msi_lock);
INIT_LIST_HEAD(&cookie->msi_page_list);
cookie->type = type;
}
@@ -95,14 +101,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
*
* Users who manage their own IOVA allocation and do not want DMA API support,
* but would still like to take advantage of automatic MSI remapping, can use
- * this to initialise their own domain appropriately. Users should reserve a
+ * this to initialise their own domain appropriately. Users may reserve a
* contiguous IOVA region, starting at @base, large enough to accommodate the
* number of PAGE_SIZE mappings necessary to cover every MSI doorbell address
- * used by the devices attached to @domain.
+ * used by the devices attached to @domain. The other way round is to provide
+ * usable iova pages through the iommu_dma_bind_doorbell API (nested stages
+ * use case)
*/
int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
{
struct iommu_dma_cookie *cookie;
+ int nesting, ret;
if (domain->type != IOMMU_DOMAIN_UNMANAGED)
return -EINVAL;
@@ -110,7 +119,12 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
if (domain->iova_cookie)
return -EEXIST;
- cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+ ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, &nesting);
+ if (!ret && nesting)
+ cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
+ else
+ cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+
if (!cookie)
return -ENOMEM;
@@ -131,6 +145,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iommu_dma_msi_page *msi, *tmp;
+ bool s2_unmap = false;
if (!cookie)
return;
@@ -138,7 +153,15 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)
put_iova_domain(&cookie->iovad);
+ if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE)
+ s2_unmap = true;
+
list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
+ if (s2_unmap && msi->phys) {
+ size_t size = cookie_msi_granule(cookie);
+
+ WARN_ON(iommu_unmap(domain, msi->gpa, size) != size);
+ }
list_del(&msi->list);
kfree(msi);
}
@@ -147,6 +170,92 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
}
EXPORT_SYMBOL(iommu_put_dma_cookie);
+/**
+ * iommu_dma_bind_guest_msi - Allows to pass the stage 1
+ * binding of a virtual MSI doorbell used by @dev.
+ *
+ * @domain: domain handle
+ * @iova: guest iova
+ * @gpa: gpa of the virtual doorbell
+ * @size: size of the granule used for the stage1 mapping
+ *
+ * In nested stage use case, the user can provide IOVA/IPA bindings
+ * corresponding to a guest MSI stage 1 mapping. When the host needs
+ * to map its own MSI doorbells, it can use @gpa as stage 2 input
+ * and map it onto the physical MSI doorbell.
+ */
+int iommu_dma_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t iova, phys_addr_t gpa, size_t size)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iommu_dma_msi_page *msi;
+ int ret = 0;
+
+ if (!cookie)
+ return -EINVAL;
+
+ if (cookie->type != IOMMU_DMA_NESTED_MSI_COOKIE)
+ return -EINVAL;
+
+ iova = iova & ~(dma_addr_t)(size - 1);
+ gpa = gpa & ~(phys_addr_t)(size - 1);
+
+ spin_lock(&cookie->msi_lock);
+
+ list_for_each_entry(msi, &cookie->msi_page_list, list) {
+ if (msi->iova == iova)
+ goto unlock; /* this page is already registered */
+ }
+
+ msi = kzalloc(sizeof(*msi), GFP_ATOMIC);
+ if (!msi) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ msi->iova = iova;
+ msi->gpa = gpa;
+ msi->s1_granule = size;
+ list_add(&msi->list, &cookie->msi_page_list);
+unlock:
+ spin_unlock(&cookie->msi_lock);
+ return ret;
+}
+EXPORT_SYMBOL(iommu_dma_bind_guest_msi);
+
+void iommu_dma_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iommu_dma_msi_page *msi;
+
+ if (!cookie)
+ return;
+
+ if (cookie->type != IOMMU_DMA_NESTED_MSI_COOKIE)
+ return;
+
+ spin_lock(&cookie->msi_lock);
+
+ list_for_each_entry(msi, &cookie->msi_page_list, list) {
+ dma_addr_t aligned_giova =
+ giova & ~(dma_addr_t)(msi->s1_granule - 1);
+
+ if (msi->iova == aligned_giova) {
+ if (msi->phys) {
+ /* unmap the stage 2 */
+ size_t size = cookie_msi_granule(cookie);
+
+ WARN_ON(iommu_unmap(domain, msi->gpa, size) != size);
+ }
+ list_del(&msi->list);
+ kfree(msi);
+ break;
+ }
+ }
+ spin_unlock(&cookie->msi_lock);
+}
+EXPORT_SYMBOL(iommu_dma_unbind_guest_msi);
+
/**
* iommu_dma_get_resv_regions - Reserved region driver helper
* @dev: Device from iommu_get_resv_regions()
@@ -1210,6 +1319,33 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
if (msi_page->phys == msi_addr)
return msi_page;
+ /*
+ * In nested stage mode, we do not allocate an MSI page in
+ * a range provided by the user. Instead, IOVA/IPA bindings are
+ * individually provided. We reuse thise IOVAs to build the
+ * GIOVA -> GPA -> MSI HPA nested stage mapping.
+ */
+ if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE) {
+ list_for_each_entry(msi_page, &cookie->msi_page_list, list)
+ if (!msi_page->phys) {
+ int ret;
+
+ /* do the stage 2 mapping */
+ ret = iommu_map(domain,
+ msi_page->gpa, msi_addr, size,
+ IOMMU_MMIO | IOMMU_WRITE);
+ if (ret) {
+ pr_warn("MSI S2 mapping failed (%d)\n",
+ ret);
+ return NULL;
+ }
+ msi_page->phys = msi_addr;
+ return msi_page;
+ }
+ pr_warn("%s no MSI binding found\n", __func__);
+ return NULL;
+ }
+
msi_page = kzalloc(sizeof(*msi_page), GFP_KERNEL);
if (!msi_page)
return NULL;
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 2112f21f73d8..f112ecdb4af6 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -12,6 +12,7 @@
#include <linux/dma-mapping.h>
#include <linux/iommu.h>
#include <linux/msi.h>
+#include <uapi/linux/iommu.h>
/* Domain management interface for IOMMU drivers */
int iommu_get_dma_cookie(struct iommu_domain *domain);
@@ -36,6 +37,9 @@ void iommu_dma_compose_msi_msg(struct msi_desc *desc,
struct msi_msg *msg);
void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
+int iommu_dma_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t iova, phys_addr_t gpa, size_t size);
+void iommu_dma_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova);
#else /* CONFIG_IOMMU_DMA */
@@ -74,6 +78,18 @@ static inline void iommu_dma_compose_msi_msg(struct msi_desc *desc,
{
}
+static inline int
+iommu_dma_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t iova, phys_addr_t gpa, size_t size)
+{
+ return -ENODEV;
+}
+
+static inline void
+iommu_dma_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova)
+{
+}
+
static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
{
}
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 10/15] iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (8 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 09/15] dma-iommu: Implement NESTED_MSI cookie Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 11/15] iommu/smmuv3: Enforce incompatibility between nested mode and HW MSI regions Eric Auger
` (4 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
In nested mode we enforce the rule that all devices belonging
to the same iommu_domain share the same msi_domain.
Indeed if there were several physical MSI doorbells being used
within a single iommu_domain, it becomes really difficult to
resolve the nested stage mapping translating into the correct
physical doorbell. So let's forbid this situation.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 41 +++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 4b796693d697..de03ac111f76 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2265,6 +2265,37 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
arm_smmu_install_ste_for_dev(master);
}
+static bool arm_smmu_share_msi_domain(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct irq_domain *irqd = dev_get_msi_domain(dev);
+ struct arm_smmu_master *master;
+ unsigned long flags;
+ bool share = false;
+
+ if (!irqd)
+ return true;
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, domain_head) {
+ struct irq_domain *d = dev_get_msi_domain(master->dev);
+
+ if (!d)
+ continue;
+ if (irqd != d) {
+ dev_info(dev, "Nested mode forbids to attach devices "
+ "using different physical MSI doorbells "
+ "to the same iommu_domain");
+ goto unlock;
+ }
+ }
+ share = true;
+unlock:
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+ return share;
+}
+
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int ret = 0;
@@ -2316,6 +2347,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
ret = -EINVAL;
goto out_unlock;
}
+ /*
+ * In nested mode we must check all devices belonging to the
+ * domain share the same physical MSI doorbell. Otherwise nested
+ * stage MSI binding is not supported.
+ */
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED &&
+ !arm_smmu_share_msi_domain(domain, dev)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
master->domain = smmu_domain;
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 11/15] iommu/smmuv3: Enforce incompatibility between nested mode and HW MSI regions
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (9 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 10/15] iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 12/15] iommu/smmuv3: Implement bind/unbind_guest_msi Eric Auger
` (3 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
Nested mode currently is not compatible with HW MSI reserved regions.
Indeed MSI transactions targeting this MSI doorbells bypass the SMMU.
Let's check nested mode is not attempted in such configuration.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 23 +++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index de03ac111f76..0c5ab4005f76 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2296,6 +2296,23 @@ static bool arm_smmu_share_msi_domain(struct iommu_domain *domain,
return share;
}
+static bool arm_smmu_has_hw_msi_resv_region(struct device *dev)
+{
+ struct iommu_resv_region *region;
+ bool has_msi_resv_region = false;
+ LIST_HEAD(resv_regions);
+
+ iommu_get_resv_regions(dev, &resv_regions);
+ list_for_each_entry(region, &resv_regions, list) {
+ if (region->type == IOMMU_RESV_MSI) {
+ has_msi_resv_region = true;
+ break;
+ }
+ }
+ iommu_put_resv_regions(dev, &resv_regions);
+ return has_msi_resv_region;
+}
+
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int ret = 0;
@@ -2350,10 +2367,12 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
/*
* In nested mode we must check all devices belonging to the
* domain share the same physical MSI doorbell. Otherwise nested
- * stage MSI binding is not supported.
+ * stage MSI binding is not supported. Also nested mode is not
+ * compatible with MSI HW reserved regions.
*/
if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED &&
- !arm_smmu_share_msi_domain(domain, dev)) {
+ (!arm_smmu_share_msi_domain(domain, dev) ||
+ arm_smmu_has_hw_msi_resv_region(dev))) {
ret = -EINVAL;
goto out_unlock;
}
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 12/15] iommu/smmuv3: Implement bind/unbind_guest_msi
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (10 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 11/15] iommu/smmuv3: Enforce incompatibility between nested mode and HW MSI regions Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 13/15] iommu/smmuv3: Report non recoverable faults Eric Auger
` (2 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
The bind/unbind_guest_msi() callbacks check the domain
is NESTED and redirect to the dma-iommu implementation.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v6 -> v7:
- remove device handle argument
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 43 +++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 0c5ab4005f76..5aa9e0e747fa 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2738,6 +2738,47 @@ static void arm_smmu_get_resv_regions(struct device *dev,
iommu_dma_get_resv_regions(dev, head);
}
+static int
+arm_smmu_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu;
+ int ret = -EINVAL;
+
+ mutex_lock(&smmu_domain->init_mutex);
+ smmu = smmu_domain->smmu;
+ if (!smmu)
+ goto out;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto out;
+
+ ret = iommu_dma_bind_guest_msi(domain, giova, gpa, size);
+out:
+ mutex_unlock(&smmu_domain->init_mutex);
+ return ret;
+}
+
+static void
+arm_smmu_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu;
+
+ mutex_lock(&smmu_domain->init_mutex);
+ smmu = smmu_domain->smmu;
+ if (!smmu)
+ goto unlock;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto unlock;
+
+ iommu_dma_unbind_guest_msi(domain, giova);
+unlock:
+ mutex_unlock(&smmu_domain->init_mutex);
+}
+
static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
struct iommu_pasid_table_config *cfg)
{
@@ -2970,6 +3011,8 @@ static struct iommu_ops arm_smmu_ops = {
.attach_pasid_table = arm_smmu_attach_pasid_table,
.detach_pasid_table = arm_smmu_detach_pasid_table,
.cache_invalidate = arm_smmu_cache_invalidate,
+ .bind_guest_msi = arm_smmu_bind_guest_msi,
+ .unbind_guest_msi = arm_smmu_unbind_guest_msi,
.dev_has_feat = arm_smmu_dev_has_feature,
.dev_feat_enabled = arm_smmu_dev_feature_enabled,
.dev_enable_feat = arm_smmu_dev_enable_feature,
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 13/15] iommu/smmuv3: Report non recoverable faults
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (11 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 12/15] iommu/smmuv3: Implement bind/unbind_guest_msi Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 14/15] iommu/smmuv3: Accept configs with more than one context descriptor Eric Auger
2020-11-16 10:43 ` [PATCH v12 15/15] iommu/smmuv3: Add PASID cache invalidation per PASID Eric Auger
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
When a stage 1 related fault event is read from the event queue,
let's propagate it to potential external fault listeners, ie. users
who registered a fault handler.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v8 -> v9:
- adapt to the removal of IOMMU_FAULT_UNRECOV_PERM_VALID:
only look at IOMMU_FAULT_UNRECOV_ADDR_VALID which comes with
perm
- do not advertise IOMMU_FAULT_UNRECOV_PASID_VALID faults for
translation faults
- trace errors if !master
- test nested before calling iommu_report_device_fault
- call the fault handler unconditionnally in non nested mode
v4 -> v5:
- s/IOMMU_FAULT_PERM_INST/IOMMU_FAULT_PERM_EXEC
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 102 +++++++++++++++++---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 80 +++++++++++++++
2 files changed, 171 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 5aa9e0e747fa..31a2500bde32 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1379,7 +1379,6 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
return 0;
}
-__maybe_unused
static struct arm_smmu_master *
arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
{
@@ -1405,25 +1404,106 @@ arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
return master;
}
+/* Populates the record fields according to the input SMMU event */
+static bool arm_smmu_transcode_fault(u64 *evt, u8 type,
+ struct iommu_fault_unrecoverable *record)
+{
+ const struct arm_smmu_fault_propagation_data *data;
+ u32 fields;
+
+ if (type >= ARRAY_SIZE(fault_propagation))
+ return false;
+
+ data = &fault_propagation[type];
+ if (!data->reason)
+ return false;
+
+ fields = data->fields;
+
+ if (data->s1_check & FIELD_GET(EVTQ_1_S2, evt[1]))
+ return false; /* S2 related fault, don't propagate */
+
+ if (fields & IOMMU_FAULT_UNRECOV_PASID_VALID)
+ record->pasid = FIELD_GET(EVTQ_0_SUBSTREAMID, evt[0]);
+ else {
+ /* all other transcoded errors have SSV */
+ if (FIELD_GET(EVTQ_0_SSV, evt[0])) {
+ record->pasid = FIELD_GET(EVTQ_0_SUBSTREAMID, evt[0]);
+ fields |= IOMMU_FAULT_UNRECOV_PASID_VALID;
+ }
+ }
+
+ if (fields & IOMMU_FAULT_UNRECOV_ADDR_VALID) {
+ if (FIELD_GET(EVTQ_1_RNW, evt[1]))
+ record->perm = IOMMU_FAULT_PERM_READ;
+ else
+ record->perm = IOMMU_FAULT_PERM_WRITE;
+ if (FIELD_GET(EVTQ_1_PNU, evt[1]))
+ record->perm |= IOMMU_FAULT_PERM_PRIV;
+ if (FIELD_GET(EVTQ_1_IND, evt[1]))
+ record->perm |= IOMMU_FAULT_PERM_EXEC;
+ record->addr = evt[2];
+ }
+
+ if (fields & IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID)
+ record->fetch_addr = FIELD_GET(EVTQ_3_FETCH_ADDR, evt[3]);
+
+ record->flags = fields;
+ record->reason = data->reason;
+ return true;
+}
+
+static void arm_smmu_report_event(struct arm_smmu_device *smmu, u64 *evt)
+{
+ u32 sid = FIELD_GET(EVTQ_0_STREAMID, evt[0]);
+ u8 type = FIELD_GET(EVTQ_0_ID, evt[0]);
+ struct arm_smmu_master *master;
+ struct iommu_fault_event event = {};
+ bool nested;
+ int i;
+
+ master = arm_smmu_find_master(smmu, sid);
+ if (!master || !master->domain)
+ goto out;
+
+ event.fault.type = IOMMU_FAULT_DMA_UNRECOV;
+
+ nested = (master->domain->stage == ARM_SMMU_DOMAIN_NESTED);
+
+ if (nested) {
+ if (arm_smmu_transcode_fault(evt, type, &event.fault.event)) {
+ /*
+ * Only S1 related faults should be reported to the
+ * guest and must not flood the host log.
+ * Also a fault handler should have been registered
+ * to guarantee the full nested functionality
+ */
+ WARN_ON_ONCE(iommu_report_device_fault(master->dev,
+ &event));
+ return;
+ }
+ } else {
+ iommu_report_device_fault(master->dev, &event);
+ }
+out:
+ dev_info(smmu->dev, "event 0x%02x received:\n", type);
+ for (i = 0; i < EVTQ_ENT_DWORDS; ++i) {
+ dev_info(smmu->dev, "\t0x%016llx\n",
+ (unsigned long long)evt[i]);
+ }
+}
+
/* IRQ and event handlers */
static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
{
- int i;
struct arm_smmu_device *smmu = dev;
struct arm_smmu_queue *q = &smmu->evtq.q;
struct arm_smmu_ll_queue *llq = &q->llq;
u64 evt[EVTQ_ENT_DWORDS];
do {
- while (!queue_remove_raw(q, evt)) {
- u8 id = FIELD_GET(EVTQ_0_ID, evt[0]);
-
- dev_info(smmu->dev, "event 0x%02x received:\n", id);
- for (i = 0; i < ARRAY_SIZE(evt); ++i)
- dev_info(smmu->dev, "\t0x%016llx\n",
- (unsigned long long)evt[i]);
-
- }
+ while (!queue_remove_raw(q, evt))
+ arm_smmu_report_event(smmu, evt);
/*
* Not much we can do on overflow, so scream and pretend we're
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index f30d0384f906..0802a3813678 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -152,6 +152,26 @@
#define ARM_SMMU_PRIQ_IRQ_CFG1 0xd8
#define ARM_SMMU_PRIQ_IRQ_CFG2 0xdc
+/* Events */
+#define ARM_SMMU_EVT_F_UUT 0x01
+#define ARM_SMMU_EVT_C_BAD_STREAMID 0x02
+#define ARM_SMMU_EVT_F_STE_FETCH 0x03
+#define ARM_SMMU_EVT_C_BAD_STE 0x04
+#define ARM_SMMU_EVT_F_BAD_ATS_TREQ 0x05
+#define ARM_SMMU_EVT_F_STREAM_DISABLED 0x06
+#define ARM_SMMU_EVT_F_TRANSL_FORBIDDEN 0x07
+#define ARM_SMMU_EVT_C_BAD_SUBSTREAMID 0x08
+#define ARM_SMMU_EVT_F_CD_FETCH 0x09
+#define ARM_SMMU_EVT_C_BAD_CD 0x0a
+#define ARM_SMMU_EVT_F_WALK_EABT 0x0b
+#define ARM_SMMU_EVT_F_TRANSLATION 0x10
+#define ARM_SMMU_EVT_F_ADDR_SIZE 0x11
+#define ARM_SMMU_EVT_F_ACCESS 0x12
+#define ARM_SMMU_EVT_F_PERMISSION 0x13
+#define ARM_SMMU_EVT_F_TLB_CONFLICT 0x20
+#define ARM_SMMU_EVT_F_CFG_CONFLICT 0x21
+#define ARM_SMMU_EVT_E_PAGE_REQUEST 0x24
+
#define ARM_SMMU_REG_SZ 0xe00
/* Common MSI config fields */
@@ -370,6 +390,15 @@
#define EVTQ_MAX_SZ_SHIFT (Q_MAX_SZ_SHIFT - EVTQ_ENT_SZ_SHIFT)
#define EVTQ_0_ID GENMASK_ULL(7, 0)
+#define EVTQ_0_SSV GENMASK_ULL(11, 11)
+#define EVTQ_0_SUBSTREAMID GENMASK_ULL(31, 12)
+#define EVTQ_0_STREAMID GENMASK_ULL(63, 32)
+#define EVTQ_1_PNU GENMASK_ULL(33, 33)
+#define EVTQ_1_IND GENMASK_ULL(34, 34)
+#define EVTQ_1_RNW GENMASK_ULL(35, 35)
+#define EVTQ_1_S2 GENMASK_ULL(39, 39)
+#define EVTQ_1_CLASS GENMASK_ULL(40, 41)
+#define EVTQ_3_FETCH_ADDR GENMASK_ULL(51, 3)
/* PRI queue */
#define PRIQ_ENT_SZ_SHIFT 4
@@ -690,6 +719,57 @@ struct arm_smmu_domain {
spinlock_t devices_lock;
};
+/* fault propagation */
+struct arm_smmu_fault_propagation_data {
+ enum iommu_fault_reason reason;
+ bool s1_check;
+ u32 fields; /* IOMMU_FAULT_UNRECOV_*_VALID bits */
+};
+
+/*
+ * Describes how SMMU faults translate into generic IOMMU faults
+ * and if they need to be reported externally
+ */
+static const struct arm_smmu_fault_propagation_data fault_propagation[] = {
+[ARM_SMMU_EVT_F_UUT] = { },
+[ARM_SMMU_EVT_C_BAD_STREAMID] = { },
+[ARM_SMMU_EVT_F_STE_FETCH] = { },
+[ARM_SMMU_EVT_C_BAD_STE] = { },
+[ARM_SMMU_EVT_F_BAD_ATS_TREQ] = { },
+[ARM_SMMU_EVT_F_STREAM_DISABLED] = { },
+[ARM_SMMU_EVT_F_TRANSL_FORBIDDEN] = { },
+[ARM_SMMU_EVT_C_BAD_SUBSTREAMID] = {IOMMU_FAULT_REASON_PASID_INVALID,
+ false,
+ IOMMU_FAULT_UNRECOV_PASID_VALID
+ },
+[ARM_SMMU_EVT_F_CD_FETCH] = {IOMMU_FAULT_REASON_PASID_FETCH,
+ false,
+ IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID
+ },
+[ARM_SMMU_EVT_C_BAD_CD] = {IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
+ false,
+ },
+[ARM_SMMU_EVT_F_WALK_EABT] = {IOMMU_FAULT_REASON_WALK_EABT, true,
+ IOMMU_FAULT_UNRECOV_ADDR_VALID |
+ IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID
+ },
+[ARM_SMMU_EVT_F_TRANSLATION] = {IOMMU_FAULT_REASON_PTE_FETCH, true,
+ IOMMU_FAULT_UNRECOV_ADDR_VALID
+ },
+[ARM_SMMU_EVT_F_ADDR_SIZE] = {IOMMU_FAULT_REASON_OOR_ADDRESS, true,
+ IOMMU_FAULT_UNRECOV_ADDR_VALID
+ },
+[ARM_SMMU_EVT_F_ACCESS] = {IOMMU_FAULT_REASON_ACCESS, true,
+ IOMMU_FAULT_UNRECOV_ADDR_VALID
+ },
+[ARM_SMMU_EVT_F_PERMISSION] = {IOMMU_FAULT_REASON_PERMISSION, true,
+ IOMMU_FAULT_UNRECOV_ADDR_VALID
+ },
+[ARM_SMMU_EVT_F_TLB_CONFLICT] = { },
+[ARM_SMMU_EVT_F_CFG_CONFLICT] = { },
+[ARM_SMMU_EVT_E_PAGE_REQUEST] = { },
+};
+
extern struct xarray arm_smmu_asid_xa;
extern struct mutex arm_smmu_asid_lock;
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 14/15] iommu/smmuv3: Accept configs with more than one context descriptor
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (12 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 13/15] iommu/smmuv3: Report non recoverable faults Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 10:43 ` [PATCH v12 15/15] iommu/smmuv3: Add PASID cache invalidation per PASID Eric Auger
14 siblings, 0 replies; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
In preparation for vSVA, let's accept userspace provided configs
with more than one CD. We check the max CD against the host iommu
capability and also the format (linear versus 2 level).
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 31a2500bde32..6549c3ee6af6 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2901,11 +2901,12 @@ static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
if (smmu_domain->s1_cfg)
goto out;
- /*
- * we currently support a single CD so s1fmt and s1dss
- * fields are also ignored
- */
- if (cfg->pasid_bits)
+ list_for_each_entry(master, &smmu_domain->devices, domain_head) {
+ if (cfg->pasid_bits > master->ssid_bits)
+ goto out;
+ }
+ if (cfg->vendor_data.smmuv3.s1fmt == STRTAB_STE_0_S1FMT_64K_L2 &&
+ !(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
goto out;
smmu_domain->s1_cfg = kzalloc(sizeof(*smmu_domain->s1_cfg),
@@ -2916,6 +2917,8 @@ static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
}
smmu_domain->s1_cfg->cdcfg.cdtab_dma = cfg->base_ptr;
+ smmu_domain->s1_cfg->s1cdmax = cfg->pasid_bits;
+ smmu_domain->s1_cfg->s1fmt = cfg->vendor_data.smmuv3.s1fmt;
smmu_domain->abort = false;
break;
default:
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v12 15/15] iommu/smmuv3: Add PASID cache invalidation per PASID
2020-11-16 10:43 [PATCH v12 00/15] SMMUv3 Nested Stage Setup (IOMMU part) Eric Auger
` (13 preceding siblings ...)
2020-11-16 10:43 ` [PATCH v12 14/15] iommu/smmuv3: Accept configs with more than one context descriptor Eric Auger
@ 2020-11-16 10:43 ` Eric Auger
2020-11-16 16:24 ` kernel test robot
14 siblings, 1 reply; 20+ messages in thread
From: Eric Auger @ 2020-11-16 10:43 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: jean-philippe, zhangfei.gao, zhangfei.gao, vivek.gautam,
shameerali.kolothum.thodi, alex.williamson, jacob.jun.pan,
yi.l.liu, tn, nicoleotsuka
In order to cascade guest CFGI_CD, let's add PASID cache invalidation
per PASID.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 6549c3ee6af6..eb0e09936803 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3002,9 +3002,19 @@ arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
} else {
return -EINVAL;
}
- }
- if (inv_info->cache & IOMMU_CACHE_INV_TYPE_PASID ||
- inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) {
+ } else if (inv_info->cache & IOMMU_CACHE_INV_TYPE_PASID) {
+ if (inv_info->granularity == IOMMU_INV_GRANU_PASID) {
+ struct iommu_inv_pasid_info *info =
+ &inv_info->granu.pasid_info;
+
+ if (!info->flags & IOMMU_INV_PASID_FLAGS_PASID)
+ return -EINVAL;
+
+ arm_smmu_sync_cd(smmu_domain, info->pasid, true);
+ } else {
+ return -ENOENT;
+ }
+ } else { /* IOMMU_CACHE_INV_TYPE_DEV_IOTLB */
return -ENOENT;
}
return 0;
--
2.21.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v12 15/15] iommu/smmuv3: Add PASID cache invalidation per PASID
2020-11-16 10:43 ` [PATCH v12 15/15] iommu/smmuv3: Add PASID cache invalidation per PASID Eric Auger
@ 2020-11-16 16:24 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2020-11-16 16:24 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, iommu, linux-kernel, kvm, kvmarm,
will, joro, maz, robin.murphy
Cc: kbuild-all, clang-built-linux
[-- Attachment #1: Type: text/plain, Size: 4934 bytes --]
Hi Eric,
I love your patch! Perhaps something to improve:
[auto build test WARNING on iommu/next]
[also build test WARNING on linus/master v5.10-rc4 next-20201116]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Eric-Auger/SMMUv3-Nested-Stage-Setup-IOMMU-part/20201116-185039
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: arm64-randconfig-r034-20201115 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c044709b8fbea2a9a375e4173a6bd735f6866c0c)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/95e4ccc61b7a7c06e1e0c6c01f362d590136ad3c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eric-Auger/SMMUv3-Nested-Stage-Setup-IOMMU-part/20201116-185039
git checkout 95e4ccc61b7a7c06e1e0c6c01f362d590136ad3c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:3010:8: warning: logical not is only applied to the left hand side of this bitwise operator [-Wlogical-not-parentheses]
if (!info->flags & IOMMU_INV_PASID_FLAGS_PASID)
^ ~
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:3010:8: note: add parentheses after the '!' to evaluate the bitwise operator first
if (!info->flags & IOMMU_INV_PASID_FLAGS_PASID)
^
( )
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:3010:8: note: add parentheses around left hand side expression to silence this warning
if (!info->flags & IOMMU_INV_PASID_FLAGS_PASID)
^
( )
1 warning generated.
vim +3010 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
2960
2961 static int
2962 arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
2963 struct iommu_cache_invalidate_info *inv_info)
2964 {
2965 struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
2966 struct arm_smmu_device *smmu = smmu_domain->smmu;
2967
2968 if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
2969 return -EINVAL;
2970
2971 if (!smmu)
2972 return -EINVAL;
2973
2974 if (inv_info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
2975 return -EINVAL;
2976
2977 if (inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB) {
2978 if (inv_info->granularity == IOMMU_INV_GRANU_PASID) {
2979 struct iommu_inv_pasid_info *info =
2980 &inv_info->granu.pasid_info;
2981
2982 if (!(info->flags & IOMMU_INV_PASID_FLAGS_ARCHID) ||
2983 (info->flags & IOMMU_INV_PASID_FLAGS_PASID))
2984 return -EINVAL;
2985
2986 __arm_smmu_tlb_inv_context(smmu_domain, info->archid);
2987
2988 } else if (inv_info->granularity == IOMMU_INV_GRANU_ADDR) {
2989 struct iommu_inv_addr_info *info = &inv_info->granu.addr_info;
2990 size_t size = info->nb_granules * info->granule_size;
2991 bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF;
2992
2993 if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID) ||
2994 (info->flags & IOMMU_INV_ADDR_FLAGS_PASID))
2995 return -EINVAL;
2996
2997 __arm_smmu_tlb_inv_range(info->addr, size,
2998 info->granule_size, leaf,
2999 smmu_domain, info->archid);
3000
3001 arm_smmu_cmdq_issue_sync(smmu);
3002 } else {
3003 return -EINVAL;
3004 }
3005 } else if (inv_info->cache & IOMMU_CACHE_INV_TYPE_PASID) {
3006 if (inv_info->granularity == IOMMU_INV_GRANU_PASID) {
3007 struct iommu_inv_pasid_info *info =
3008 &inv_info->granu.pasid_info;
3009
> 3010 if (!info->flags & IOMMU_INV_PASID_FLAGS_PASID)
3011 return -EINVAL;
3012
3013 arm_smmu_sync_cd(smmu_domain, info->pasid, true);
3014 } else {
3015 return -ENOENT;
3016 }
3017 } else { /* IOMMU_CACHE_INV_TYPE_DEV_IOTLB */
3018 return -ENOENT;
3019 }
3020 return 0;
3021 }
3022
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39466 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread