diff mbox series

iommu/arm-smmu-v3: Add a configurable tlbi_range_max_n_shift for TLBI

Message ID 20230814215701.5455-1-nicolinc@nvidia.com (mailing list archive)
State New, archived
Headers show
Series iommu/arm-smmu-v3: Add a configurable tlbi_range_max_n_shift for TLBI | expand

Commit Message

Nicolin Chen Aug. 14, 2023, 9:57 p.m. UTC 
When receiving an __arm_smmu_tlb_inv_range() call with a large size, there
could be a long latency at this function call: one part is coming from a
large software overhead in the routine of building commands, and the other
part is coming from CMDQ hardware consuming the large number of commands.
This latency could be significantly large on an SMMU that does not support
range invalidation commands, i.e. no ARM_SMMU_FEAT_RANGE_INV.

One way to optimize this is to replace a large number of VA invalidation
commands with one single per-asid invalidation command, when the requested
size is above a threshold. This threshold can be configurable depending on
the SMMU implementaion, and its default value can be VA_BITS, so it'd even
make sense for SMMUs with ARM_SMMU_FEAT_RANGE_INV to do such a replacement.

Add a tlbi_range_max_n_shift threshold per SMMU device and allow it to be
configured via an SYSFS node, and then add an ABI doc for this new node.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 .../ABI/testing/sysfs-class-iommu-arm-smmu-v3 |  8 +++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 54 ++++++++++++++++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  3 ++
 3 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-iommu-arm-smmu-v3

Comments

Jason Gunthorpe Aug. 16, 2023, 11:59 a.m. UTC | #1 
On Mon, Aug 14, 2023 at 02:57:01PM -0700, Nicolin Chen wrote:
> +static ssize_t tlbi_range_max_n_shift_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct arm_smmu_device *smmu = dev_get_drvdata(dev->parent);
> +
> +	return sprintf(buf, "%u\n", smmu->tlbi_range_max_n_shift);
> +}

sysfs_emit and missing newline

> +static ssize_t tlbi_range_max_n_shift_store(struct device *dev,
> +					    struct device_attribute *attr,
> +					    const char *buf, size_t size)
> +{
> +	struct arm_smmu_device *smmu = dev_get_drvdata(dev->parent);
> +	unsigned int max_n_shift;
> +	int ret;
> +
> +	ret = kstrtou32(buf, 0, &max_n_shift);
> +	if (ret)
> +		return ret;
> +	if (max_n_shift > VA_BITS || max_n_shift < PAGE_SHIFT)
> +		return -EINVAL;
> +	smmu->tlbi_range_max_n_shift = max_n_shift;
> +	return size;

This seems convoluted for a uapi, you should just make it
'invalidate_threshold' in bytes or something simpler.

> +
> +static struct attribute_group arm_smmu_group = {
> +	.name = "arm-smmu-v3",
> +	.attrs = arm_smmu_attrs,
> +};

Do we really need the subdirectory?

Jason
Nicolin Chen Aug. 16, 2023, 5:09 p.m. UTC | #2 
On Wed, Aug 16, 2023 at 08:59:59AM -0300, Jason Gunthorpe wrote:
> On Mon, Aug 14, 2023 at 02:57:01PM -0700, Nicolin Chen wrote:
> > +static ssize_t tlbi_range_max_n_shift_show(struct device *dev,
> > +					   struct device_attribute *attr,
> > +					   char *buf)
> > +{
> > +	struct arm_smmu_device *smmu = dev_get_drvdata(dev->parent);
> > +
> > +	return sprintf(buf, "%u\n", smmu->tlbi_range_max_n_shift);
> > +}
> 
> sysfs_emit and missing newline

Will replace and add a new line.

> > +static ssize_t tlbi_range_max_n_shift_store(struct device *dev,
> > +					    struct device_attribute *attr,
> > +					    const char *buf, size_t size)
> > +{
> > +	struct arm_smmu_device *smmu = dev_get_drvdata(dev->parent);
> > +	unsigned int max_n_shift;
> > +	int ret;
> > +
> > +	ret = kstrtou32(buf, 0, &max_n_shift);
> > +	if (ret)
> > +		return ret;
> > +	if (max_n_shift > VA_BITS || max_n_shift < PAGE_SHIFT)
> > +		return -EINVAL;
> > +	smmu->tlbi_range_max_n_shift = max_n_shift;
> > +	return size;
> 
> This seems convoluted for a uapi, you should just make it
> 'invalidate_threshold' in bytes or something simpler.

Hmm. That'd be a direct 64-bit size configuration, and very likely
to be at upper 32-bit range, so I feel the value would be very big
that might be hard to read.

With that said, I can rename the uAPI, yet still keep the bitshift
configuration.

> > +static struct attribute_group arm_smmu_group = {
> > +	.name = "arm-smmu-v3",
> > +	.attrs = arm_smmu_attrs,
> > +};
> 
> Do we really need the subdirectory?

OK. Let me try unfolding it.

Thanks!
Nicolin
Jason Gunthorpe Aug. 16, 2023, 5:34 p.m. UTC | #3 
On Wed, Aug 16, 2023 at 10:09:20AM -0700, Nicolin Chen wrote:
> > This seems convoluted for a uapi, you should just make it
> > 'invalidate_threshold' in bytes or something simpler.
> 
> Hmm. That'd be a direct 64-bit size configuration, and very likely
> to be at upper 32-bit range, so I feel the value would be very big
> that might be hard to read.

hard to read is fine, it is sysfs

Jason
Nicolin Chen Aug. 16, 2023, 5:37 p.m. UTC | #4 
On Wed, Aug 16, 2023 at 02:34:33PM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 16, 2023 at 10:09:20AM -0700, Nicolin Chen wrote:
> > > This seems convoluted for a uapi, you should just make it
> > > 'invalidate_threshold' in bytes or something simpler.
> > 
> > Hmm. That'd be a direct 64-bit size configuration, and very likely
> > to be at upper 32-bit range, so I feel the value would be very big
> > that might be hard to read.
> 
> hard to read is fine, it is sysfs

OK. I'd just do it in size then.

Thanks
Nic
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-class-iommu-arm-smmu-v3 b/Documentation/ABI/testing/sysfs-class-iommu-arm-smmu-v3
new file mode 100644
index 000000000000..ed1a8ead82d3
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-iommu-arm-smmu-v3
@@ -0,0 +1,8 @@ 
+What:		/sys/class/iommu/<iommu>/arm-smmu-v3/tlbi_range_max_n_shift
+Date:		August 2023
+KernelVersion:	6.6
+Contact:	Nicolin Chen <nicolinc@nvidia.com>
+Description:
+		Max number of shift to set up a threshold for SMMU to replace
+		VA invalidation commands with a single per-asid invalidation.
+		Format: %u
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 9b0dc3505601..e52eb8a0ffdf 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1886,11 +1886,21 @@  static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	unsigned long end = iova + size, num_pages = 0, tg = 0;
 	size_t inv_range = granule;
+	size_t max_size = 1UL << smmu->tlbi_range_max_n_shift;
 	struct arm_smmu_cmdq_batch cmds;
 
 	if (!size)
 		return;
 
+	/*
+	 * Convert a large range/number of VA invalidation(s) to one single ASID
+	 * invalidation, when the input size is greater than the threshold. This
+	 * simplifies the command building routine, espeicaly on an SMMU without
+	 * ARM_SMMU_FEAT_RANGE_INV.
+	 */
+	if (cmd->tlbi.asid && size >= max_size)
+		return arm_smmu_tlb_inv_asid(smmu, cmd->tlbi.asid);
+
 	if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
 		/* Get the leaf page size */
 		tg = __ffs(smmu_domain->domain.pgsize_bitmap);
@@ -3107,6 +3117,7 @@  static int arm_smmu_init_structures(struct arm_smmu_device *smmu)
 {
 	int ret;
 
+	smmu->tlbi_range_max_n_shift = VA_BITS;
 	mutex_init(&smmu->streams_mutex);
 	smmu->streams = RB_ROOT;
 
@@ -3808,6 +3819,47 @@  static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
 	iort_put_rmr_sids(dev_fwnode(smmu->dev), &rmr_list);
 }
 
+static ssize_t tlbi_range_max_n_shift_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct arm_smmu_device *smmu = dev_get_drvdata(dev->parent);
+
+	return sprintf(buf, "%u\n", smmu->tlbi_range_max_n_shift);
+}
+static ssize_t tlbi_range_max_n_shift_store(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t size)
+{
+	struct arm_smmu_device *smmu = dev_get_drvdata(dev->parent);
+	unsigned int max_n_shift;
+	int ret;
+
+	ret = kstrtou32(buf, 0, &max_n_shift);
+	if (ret)
+		return ret;
+	if (max_n_shift > VA_BITS || max_n_shift < PAGE_SHIFT)
+		return -EINVAL;
+	smmu->tlbi_range_max_n_shift = max_n_shift;
+	return size;
+}
+static DEVICE_ATTR_RW(tlbi_range_max_n_shift);
+
+static struct attribute *arm_smmu_attrs[] = {
+	&dev_attr_tlbi_range_max_n_shift.attr,
+	NULL,
+};
+
+static struct attribute_group arm_smmu_group = {
+	.name = "arm-smmu-v3",
+	.attrs = arm_smmu_attrs,
+};
+
+static const struct attribute_group *arm_smmu_groups[] = {
+	&arm_smmu_group,
+	NULL,
+};
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	int irq, ret;
@@ -3900,7 +3952,7 @@  static int arm_smmu_device_probe(struct platform_device *pdev)
 		return ret;
 
 	/* And we're up. Go go go! */
-	ret = iommu_device_sysfs_add(&smmu->iommu, dev, NULL,
+	ret = iommu_device_sysfs_add(&smmu->iommu, dev, arm_smmu_groups,
 				     "smmu3.%pa", &ioaddr);
 	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 dcab85698a4e..07636d596901 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -665,6 +665,9 @@  struct arm_smmu_device {
 	unsigned long			oas; /* PA */
 	unsigned long			pgsize_bitmap;
 
+	/* Threshold to convert VA range TLBI to asid TLBI */
+	unsigned int			tlbi_range_max_n_shift;
+
 #define ARM_SMMU_MAX_ASIDS		(1 << 16)
 	unsigned int			asid_bits;