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

From: Nicolin Chen <[email-masked]>
Date: Tue, 26 May 2026 19:18:54 -0700

On Wed, May 20, 2026 at 02:37:09PM +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;
> +	guard(mutex)(&smmu->streams_mutex);

Given the whole thing is fenced with streams_mutex..

> +	sid = ctx->sid;

... the point in having the ctx is for the SID?

SID is convertible from the parent folder name. Can you try:

	struct file *file = seq->file;
	struct dentry *dentry = file->f_path.dentry;
	const char *str_sid = dentry->d_parent->d_name.name;

	if (kstrtoint(str_sid, 0, &sid)) {
		seq_puts(seq, "Error parsing SID string from: %s\n", str_sid);
		return 0;
	}
?

> +/**
> + * arm_smmu_debugfs_create_stream_table() - Create debugfs entries for stream table
> + * @smmu: SMMU device
> + * @dev: device to create entries for
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +int arm_smmu_debugfs_create_stream_table(struct arm_smmu_device *smmu,
> +					 struct device *dev)
> +{
> +	struct dentry *stream_dir, *dev_dir;
> +	struct arm_smmu_master *master;
> +	struct ste_context *ctx;
> +	char name[64];
> +	u32 sid;
> +	int i;
[...]
> +	master = dev_iommu_priv_get(dev);
> +	if (!master || !master->num_streams)
> +		return -ENODEV;

The caller has master. Why not just pass it in? Then, you would
have master->smmu and master->dev.

> +/**
> + * arm_smmu_debugfs_remove_stream_table() - Remove debugfs entries for stream table
> + * @smmu: SMMU device
> + * @dev: device to remove entries for
> + *
> + * This function removes the debugfs directories created by
> + * arm_smmu_debugfs_create_stream_table().
> + */
> +void arm_smmu_debugfs_remove_stream_table(struct arm_smmu_device *smmu,
> +					  struct device *dev)
> +{
> +	struct dentry *stream_dir, *dev_dir;
> +	struct arm_smmu_master *master;
> +	struct ste_context *ctx, *tmp;
> +	char name[64];
> +	int i;
> +
> +	/* Check if stream_table directory exists */
> +	if (!smmu->debugfs || !smmu->debugfs->stream_dir)
> +		return;
> +
> +	stream_dir = smmu->debugfs->stream_dir;
> +	master = dev_iommu_priv_get(dev);
> +	if (!master)
> +		return;

Ditto.

> 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 8e1c19b6831c..9c9be63dc9e2 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -821,12 +821,29 @@ struct arm_smmu_impl_ops {
>  };
>  
>  #ifdef CONFIG_ARM_SMMU_V3_DEBUGFS
> +struct ste_context {
> +	struct arm_smmu_master		*master;
> +	u32				sid;
> +	struct list_head		node;
> +};

Does this need to be in the header?

Nicolin