Re: [PATCH v2 4/8] iommu/arm-smmu-v3: move stall_enabled to the cd table

[Jason Gunthorpe]

On Mon, Jul 31, 2023 at 06:48:14PM +0800, Michael Shavit wrote:
> This controls whether CD entries will have the stall bit set when
> writing entries into the table.
> 
> Signed-off-by: Michael Shavit <mshavit@xxxxxxxxxx>
> ---
> 
> Changes in v2:
> - Use a bitfield instead of a bool for stall_enabled
> 
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 ++++----
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 ++-
>  2 files changed, 6 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 8a286e3838d70..654acf6002bf3 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1114,7 +1114,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
>  			FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
>  			CTXDESC_CD_0_V;
>  
> -		if (smmu_domain->stall_enabled)
> +		if (smmu_domain->cd_table.stall_enabled)
>  			val |= CTXDESC_CD_0_S;
>  	}

Since patch 6 makes arm_smmu_write_ctx_desc() take in the master
parameter, it does make sense to just refer to the stall in the master
at this point. Can you defer this until after patch 6?

> 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 35a93e8858872..05b1f0ee60808 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -597,6 +597,8 @@ struct arm_smmu_ctx_desc_cfg {
>  	unsigned int			num_l1_ents;
>  	/* log2 of the maximum number of CDs supported by this table */
>  	u8				max_cds_bits;
> +	/* Whether CD entries in this table have the stall bit set. */
> +	u8				stall_enabled:1;
>  };
>  
>  struct arm_smmu_s2_cfg {
> @@ -714,7 +716,6 @@ struct arm_smmu_domain {
>  	struct mutex			init_mutex; /* Protects smmu pointer */
>  
>  	struct io_pgtable_ops		*pgtbl_ops;
> -	bool				stall_enabled;
>  	atomic_t			nr_ats_masters;
>  
>  	enum arm_smmu_domain_stage	stage;

But this also makes sense, and removing stall_enabled from the domain
is important, so

Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>

If you keep it this way

Jason