linux-kernel - Re: [PATCH v4 1/4] iommu/arm-smmu: Call configuration impl hook before consuming features

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:   Wed, 15 Jul 2020 11:20:37 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Tomasz Nowicki <tn@...ihalf.com>, will@...nel.org, joro@...tes.org,
        gregory.clement@...tlin.com, robh+dt@...nel.org, hannah@...vell.com
Cc:     linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
        devicetree@...r.kernel.org, catalin.marinas@....com,
        nadavh@...vell.com, linux-arm-kernel@...ts.infradead.org,
        mw@...ihalf.com
Subject: Re: [PATCH v4 1/4] iommu/arm-smmu: Call configuration impl hook
 before consuming features

On 2020-07-15 08:06, Tomasz Nowicki wrote:
> 'cfg_probe' hook is called at the very end of configuration probing
> procedure and therefore features override and workaround may become
> complex like for ID register fixups. In preparation for adding Marvell
> errata move 'cfg_probe' a bit earlier to have chance to adjust
> the detected features before we start consuming them.
> 
> Since the Cavium quirk (the only user) does not alter features
> it is safe to do so.

Sorry for the confusion of failing to match my own intent in the first 
place ;)

Reviewed-by: Robin Murphy <robin.murphy@....com>

> Suggested-by: Robin Murphy <robin.murphy@....com>
> Signed-off-by: Tomasz Nowicki <tn@...ihalf.com>
> ---
>   drivers/iommu/arm-smmu.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 243bc4cb2705..19f906de6420 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1728,7 +1728,7 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
>   	unsigned int size;
>   	u32 id;
>   	bool cttw_reg, cttw_fw = smmu->features & ARM_SMMU_FEAT_COHERENT_WALK;
> -	int i;
> +	int i, ret;
>   
>   	dev_notice(smmu->dev, "probing hardware configuration...\n");
>   	dev_notice(smmu->dev, "SMMUv%d with:\n",
> @@ -1891,6 +1891,12 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
>   			smmu->features |= ARM_SMMU_FEAT_FMT_AARCH64_64K;
>   	}
>   
> +	if (smmu->impl && smmu->impl->cfg_probe) {
> +		ret = smmu->impl->cfg_probe(smmu);
> +		if (ret)
> +			return ret;
> +	}
> +
>   	/* Now we've corralled the various formats, what'll it do? */
>   	if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_S)
>   		smmu->pgsize_bitmap |= SZ_4K | SZ_64K | SZ_1M | SZ_16M;
> @@ -1918,9 +1924,6 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
>   		dev_notice(smmu->dev, "\tStage-2: %lu-bit IPA -> %lu-bit PA\n",
>   			   smmu->ipa_size, smmu->pa_size);
>   
> -	if (smmu->impl && smmu->impl->cfg_probe)
> -		return smmu->impl->cfg_probe(smmu);
> -
>   	return 0;
>   }
>   
>