linux-kernel - Re: [PATCH] soc: qcom: llcc: Fix dis_cap_alloc and retain_on

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: <20231103193345.GY3553829@hu-bjorande-lv.qualcomm.com>
Date:   Fri, 3 Nov 2023 12:33:45 -0700
From:   Bjorn Andersson <quic_bjorande@...cinc.com>
To:     Atul Dhudase <quic_adhudase@...cinc.com>
CC:     <agross@...nel.org>, <andersson@...nel.org>,
        <konrad.dybcio@...aro.org>, <swboyd@...omium.org>,
        <isaacm@...eaurora.org>, <dianders@...omium.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] soc: qcom: llcc: Fix dis_cap_alloc and retain_on_pc
 configuration

On Fri, Nov 03, 2023 at 04:27:12PM +0530, Atul Dhudase wrote:
> While programming dis_cap_alloc and retain_on_pc, set a bit
> corresponding to a specific SCID without disturbing the
> previously configured bits.
> 

As far as I can see, the only invocation of _qcom_llcc_cfg_program()
comes from qcom_llcc_cfg_program(), which is only called once, from
qcom_llcc_probe(), and here also seems to only be the single write to
these two registers.

This implies that "the previously configured bits" would be some unknown
configuration provided to us either from the bootloader or by reset of
the hardware. As such this changes the value of the two registers from
being known, to having 31 unknown bits.


I'm not saying that the change is wrong, but you're altering the
behavior of every platform except SDM845.
As such, I want the commit message to provide an actual problem
description, and mention the fact that you're changing the logic to
retain the state prior to Linux.

Regards,
Bjorn

> Fixes: c14e64b46944 ("soc: qcom: llcc: Support chipsets that can write to llcc")
> Signed-off-by: Atul Dhudase <quic_adhudase@...cinc.com>
> ---
>  drivers/soc/qcom/llcc-qcom.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 674abd0d6700..509d972c1bd9 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -941,15 +941,15 @@ static int _qcom_llcc_cfg_program(const struct llcc_slice_config *config,
>  		u32 disable_cap_alloc, retain_pc;
> 
>  		disable_cap_alloc = config->dis_cap_alloc << config->slice_id;
> -		ret = regmap_write(drv_data->bcast_regmap,
> -				LLCC_TRP_SCID_DIS_CAP_ALLOC, disable_cap_alloc);
> +		ret = regmap_update_bits(drv_data->bcast_regmap, LLCC_TRP_SCID_DIS_CAP_ALLOC,
> +				BIT(config->slice_id), disable_cap_alloc);
>  		if (ret)
>  			return ret;
> 
>  		if (drv_data->version < LLCC_VERSION_4_1_0_0) {
>  			retain_pc = config->retain_on_pc << config->slice_id;
> -			ret = regmap_write(drv_data->bcast_regmap,
> -					LLCC_TRP_PCB_ACT, retain_pc);
> +			ret = regmap_update_bits(drv_data->bcast_regmap, LLCC_TRP_PCB_ACT,
> +					BIT(config->slice_id), retain_pc);
>  			if (ret)
>  				return ret;
>  		}
> --
> 2.25.1
>