linux-kernel - Re: [PATCH] iommu/arm-smmu-qcom: Rework the logic finding the bypass quirk

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: <20230210174755.la6rj3m3a33d53lf@ripper>
Date:   Fri, 10 Feb 2023 09:47:55 -0800
From:   Bjorn Andersson <andersson@...nel.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc:     will@...nel.org, joro@...tes.org, robin.murphy@....com,
        johan+linaro@...nel.org, steev@...i.org,
        linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iommu/arm-smmu-qcom: Rework the logic finding the bypass
 quirk

On Wed, Feb 01, 2023 at 01:55:00PM +0530, Manivannan Sadhasivam wrote:
> The logic used to find the quirky firmware that intercepts the writes to
> S2CR register to replace bypass type streams with a fault, and ignore the
> fault type, is not working with the firmware on newer SoCs like SC8280XP.
> 
> The current logic uses the last stream mapping group (num_mapping_groups
> - 1) as an index for finding quirky firmware. But on SC8280XP, this
> logic is not working as the number of stream mapping groups reported by
> the SMMU (163 as on the SC8280XP-CRD device) is not valid for some reason.
> So the current logic that checks the (163-1) S2CR entry fails to detect
> the quirky firmware on these devices and triggers invalid context fault
> for bypass streams.
> 
> To fix this issue, rework the logic to find the first non-valid (free)
> stream mapping register group (SMR) and use that index to access S2CR
> for detecting the bypass quirk.
> 
> This also warrants a change in variable name from last_s2cr to free_s2cr.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>

Reviewed-by: Bjorn Andersson <andersson@...nel.org>

Regards,
Bjorn

> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 24 +++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 78fc0e1bf215..4104f81b8d8f 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -267,23 +267,37 @@ static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>  
>  static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
>  {
> -	unsigned int last_s2cr = ARM_SMMU_GR0_S2CR(smmu->num_mapping_groups - 1);
>  	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
> +	u32 free_s2cr;
>  	u32 reg;
>  	u32 smr;
>  	int i;
>  
> +	/*
> +	 * Find the first non-valid (free) stream mapping register group and
> +	 * use that index to access S2CR for detecting the bypass quirk.
> +	 */
> +	for (i = 0; i < smmu->num_mapping_groups; i++) {
> +		smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i));
> +
> +		if (!FIELD_GET(ARM_SMMU_SMR_VALID, smr))
> +			break;
> +	}
> +
> +	free_s2cr = ARM_SMMU_GR0_S2CR(i);
> +
>  	/*
>  	 * With some firmware versions writes to S2CR of type FAULT are
>  	 * ignored, and writing BYPASS will end up written as FAULT in the
> -	 * register. Perform a write to S2CR to detect if this is the case and
> -	 * if so reserve a context bank to emulate bypass streams.
> +	 * register. Perform a write to the first free S2CR to detect if
> +	 * this is the case and if so reserve a context bank to emulate
> +	 * bypass streams.
>  	 */
>  	reg = FIELD_PREP(ARM_SMMU_S2CR_TYPE, S2CR_TYPE_BYPASS) |
>  	      FIELD_PREP(ARM_SMMU_S2CR_CBNDX, 0xff) |
>  	      FIELD_PREP(ARM_SMMU_S2CR_PRIVCFG, S2CR_PRIVCFG_DEFAULT);
> -	arm_smmu_gr0_write(smmu, last_s2cr, reg);
> -	reg = arm_smmu_gr0_read(smmu, last_s2cr);
> +	arm_smmu_gr0_write(smmu, free_s2cr, reg);
> +	reg = arm_smmu_gr0_read(smmu, free_s2cr);
>  	if (FIELD_GET(ARM_SMMU_S2CR_TYPE, reg) != S2CR_TYPE_BYPASS) {
>  		qsmmu->bypass_quirk = true;
>  		qsmmu->bypass_cbndx = smmu->num_context_banks - 1;
> -- 
> 2.25.1
>