linux-kernel - Re: [PATCH v2 4/8] iommu/arm-smmu-v3: move stall

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 1 Aug 2023 11:12:44 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Michael Shavit <mshavit@...gle.com>
Cc:     iommu@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, will@...nel.org,
        robin.murphy@....com, nicolinc@...dia.com, jean-philippe@...aro.org
Subject: Re: [PATCH v2 4/8] iommu/arm-smmu-v3: move stall_enabled to the cd
 table

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@...gle.com>
> ---
> 
> 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@...dia.com>

If you keep it this way

Jason