Re: [RFC PATCH v2 3/5] iommu/arm-smmu-v3: Add Stream Table Entry display to debugfs

[Nicolin Chen <nicolinc@nvidia.com>]

On Sat, Mar 28, 2026 at 06:17:04PM +0800, Qinxin Xia wrote:
> +static int smmu_debugfs_ste_show(struct seq_file *seq, void *unused)
> +{
> +	struct ste_context *ctx = seq->private;
> +	struct arm_smmu_master *master = ctx->master;
> +	struct arm_smmu_device *smmu;
> +	struct arm_smmu_ste *ste;
> +	u32 sid, cfg;
> +	int i;
> +
> +	if (!master) {
> +		seq_puts(seq, "No SMMU master data\n");
> +		return 0;
> +	}
> +
> +	smmu = master->smmu;
> +	scoped_guard(mutex, &smmu->streams_mutex) {

Instead:
	guard(mutex)(&smmu->streams_mutex);

> +		sid = ctx->sid;
> +
> +		if (!arm_smmu_sid_in_range(smmu, sid)) {
> +			seq_printf(seq, "Invalid Stream ID: %u (max %u)\n",
> +				   sid, (1 << smmu->sid_bits) - 1);
> +			return 0;
> +		}
> +
> +		ste = arm_smmu_get_step_for_sid(smmu, sid);
> +		if (!ste) {
> +			seq_printf(seq, "STE not available for SID %u\n", sid);
> +			return 0;
> +		}
> +
> +		seq_printf(seq, "STE for Stream ID %u\n", sid);
> +		seq_printf(seq, "  Valid: %s\n",
> +			   le64_to_cpu(ste->data[0]) & STRTAB_STE_0_V ? "Yes" : "No");
> +
> +		seq_puts(seq, "  Config: ");
> +
> +		cfg = FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(ste->data[0]));
> +
> +		switch (cfg) {
> +		case STRTAB_STE_0_CFG_BYPASS:
> +			seq_puts(seq, "BYPASS\n");
> +			break;
> +		case STRTAB_STE_0_CFG_S1_TRANS:
> +			seq_puts(seq, "only S1_TRANS\n");
> +			break;
> +		case STRTAB_STE_0_CFG_S2_TRANS:
> +			seq_puts(seq, "only S2_TRANS\n");
> +			break;
> +		case STRTAB_STE_0_CFG_NESTED:
> +			seq_puts(seq, "S1+S2_TRANS\n");
> +			break;
> +		case STRTAB_STE_0_CFG_ABORT:
> +			seq_puts(seq, "ABORT\n");
> +			break;
> +		default:
> +			seq_puts(seq, "UNKNOWN\n");
> +		}
> +
> +		if (le64_to_cpu(ste->data[0]) & STRTAB_STE_0_CFG_S1_TRANS) {
> +			seq_printf(seq, "  S1ContextPtr: 0x%016llx\n",
> +				   le64_to_cpu(ste->data[1]) & STRTAB_STE_0_S1CTXPTR_MASK);
> +		}
> +
> +		if (le64_to_cpu(ste->data[0]) & STRTAB_STE_0_CFG_S2_TRANS) {
> +			seq_printf(seq, "  S2ContextPtr: 0x%016llx\n",
> +				   le64_to_cpu(ste->data[3]) & STRTAB_STE_3_S2TTB_MASK);
> +		}
> +
> +		/* Display raw STE data */
> +		seq_puts(seq, "  Raw Data:\n");
> +		for (i = 0; i < STRTAB_STE_DWORDS; i++)
> +			seq_printf(seq, "    STE[%d]: 0x%016llx\n", i,
> +				   le64_to_cpu(ste->data[i]));
> +	}
> +		return 0;

Check the indentation of the return line.

> +/**
> + * arm_smmu_debugfs_create_stream_table() - Create debugfs entries for stream table
> + * @dev: device to create entries for
> + * @smmu: SMMU device
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +int arm_smmu_debugfs_create_stream_table(struct device *dev,
> +					 struct arm_smmu_device *smmu)

Usually @smmu would be the first parameter.

> +{
> +	struct dentry *stream_dir, *dev_dir;
> +	struct arm_smmu_master *master;
> +	struct ste_context *ctx;
> +	char name[64];
> +	u32 sid;
> +	int i;
> +
> +	scoped_guard(mutex, &arm_smmu_debugfs_lock) {
> +		if (!smmu->debugfs->stream_dir) {
> +			stream_dir = debugfs_create_dir("stream_table",
> +							smmu->debugfs->smmu_dir);
> +			if (!stream_dir)
> +				return -ENOMEM;
> +
> +			smmu->debugfs->stream_dir = stream_dir;
> +		} else {
> +			stream_dir = smmu->debugfs->stream_dir;
> +		}
> +	}
> +
> +	master = dev_iommu_priv_get(dev);
> +	if (!master || !master->num_streams)
> +		return -ENODEV;
> +
> +	for (i = 0; i < master->num_streams; i++) {
> +		sid = master->streams[i].id;
> +		snprintf(name, sizeof(name), "%u", sid);
> +		dev_dir = debugfs_create_dir(name, stream_dir);
> +		if (!dev_dir)
> +			continue;
> +
> +		/* Create STE file */
> +		ctx = kzalloc_obj(*ctx);
> +		ctx->master = master;
> +		ctx->sid = sid;
> +		spin_lock(&smmu->debugfs->stream_lock);
> +		list_add_tail(&ctx->node, &smmu->debugfs->stream_list);
> +		spin_unlock(&smmu->debugfs->stream_lock);

May consider an RCU list instead of locking.

> +/**
> + * arm_smmu_debugfs_remove_stream_table() - Remove debugfs entries for stream table
> + * @dev: device to remove entries for
> + * @smmu: SMMU device

Again, @smmu could be the first parameter.

> + * This function removes the debugfs directories created by
> + * arm_smmu_debugfs_create_stream_table().
> + */
> +void arm_smmu_debugfs_remove_stream_table(struct device *dev,
> +				      struct arm_smmu_device *smmu)

Please double check the indentation. It looks odd on my side.

> -static struct arm_smmu_ste *
> +#ifndef CONFIG_ARM_SMMU_V3_DEBUGFS
> +static
> +#endif
> +struct arm_smmu_ste *
>  arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid)

Could probably move this to the header.

> -static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
> +#ifndef CONFIG_ARM_SMMU_V3_DEBUGFS
> +static
> +#endif
> +bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)

Ditto

> @@ -3648,10 +3658,14 @@ static void arm_smmu_release_device(struct device *dev)
>  
>  	WARN_ON(master->iopf_refcount);
>  
> +#ifdef CONFIG_ARM_SMMU_V3_DEBUGFS
> +	arm_smmu_debugfs_remove_stream_table(dev, master->smmu);
> +#endif
>  	arm_smmu_disable_pasid(master);
>  	arm_smmu_remove_master(master);
>  	if (arm_smmu_cdtab_allocated(&master->cd_table))
>  		arm_smmu_free_cd_tables(master);
> +
>  	kfree(master);

Meaningless line.

>  struct arm_smmu_debugfs {
> +	struct list_head		stream_list;
> +	spinlock_t			stream_lock;
>  	struct dentry			*smmu_dir;
> +	struct dentry			*stream_dir;
>  	/* Reserved for future extensions */
>  };

That's the end of the struct. What do you reserve?

Nicolin