linux-kernel - Re: [PATCH v2 2/8] iommu/arm-smmu-v3: Replace s1_cfg with cdtab

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]

Message-ID: <ZMkMF4e7w1GTln7s@nvidia.com>
Date:   Tue, 1 Aug 2023 10:43:51 -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 2/8] iommu/arm-smmu-v3: Replace s1_cfg with cdtab_cfg

On Mon, Jul 31, 2023 at 06:48:12PM +0800, Michael Shavit wrote:
> arm_smmu_ctx_desc_cfg is renamed to arm_smmu_cdtab_cfg to make it more
> obvious that it represents a cd table. The max number of CDs that can be
> represented by the CD table is stored in this truct in its log2 form
> since it is more useful for users of the CD table, and replaces the
> s1cdmax field in s1_cfg. Instead of storing s1_cfg.s1fmt, it can also be
> trivially computed from the cdtab_cfg, and is therefore removed from
> s1_cfg.

To Nicolin's suggestion, how about

Remove struct arm_smmu_s1_cfg. This is really just a CD table with a
bit of extra information. Enhance the existing CD table structure,
struct arm_smmu_ctx_desc_cfg, with max_cds_bits and replace all usage
of arm_smmu_s1_cfg with arm_smmu_ctx_desc_cfg.

Compute the other values that were stored in s1cfg directly from
existing arm_smmu_ctx_desc_cfg.

For clarity the name "cd_table" for the variables pointing to
arm_smmu_ctx_desc_cfg in the new code.  A later patch will make this
fully consistent.

> @@ -2082,11 +2083,11 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
>  
>  	/* Free the CD and ASID, if we allocated them */
>  	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> -		struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> +		struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;

Use cd_table here
  
>  		/* Prevent SVA from touching the CD while we're freeing it */
>  		mutex_lock(&arm_smmu_asid_lock);
> -		if (cfg->cdcfg.cdtab)
> +		if (cdcfg->cdtab)
>  			arm_smmu_free_cd_tables(smmu_domain);
>  		arm_smmu_free_asid(&smmu_domain->cd);
>  		mutex_unlock(&arm_smmu_asid_lock);
> @@ -2106,7 +2107,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
>  	int ret;
>  	u32 asid;
>  	struct arm_smmu_device *smmu = smmu_domain->smmu;
> -	struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> +	struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;

And here

Jason