linux-kernel - Re: [PATCH v3 2/4] soc: qcom: llcc-qcom: Add support for LLCC V6

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: <92b6ea9d-0b13-472f-afad-2b67d869a0bb@quicinc.com>
Date: Wed, 9 Apr 2025 14:16:13 -0700
From: Melody Olvera <quic_molvera@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Bjorn Andersson
	<andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Conor Dooley
	<conor@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        "Satya Durga
 Srinivasu Prabhala" <quic_satyap@...cinc.com>,
        Trilok Soni
	<quic_tsoni@...cinc.com>
CC: <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/4] soc: qcom: llcc-qcom: Add support for LLCC V6



On 3/26/2025 6:39 AM, Konrad Dybcio wrote:
> On 3/24/25 9:29 PM, Melody Olvera wrote:
>> Add support for LLCC V6. V6 adds several additional usecase IDs,
>> rearrages several registers and offsets, and supports slice IDs
>> over 31, so add a new function for programming LLCC V6.
>>
>> Signed-off-by: Melody Olvera <quic_molvera@...cinc.com>
>> ---
> [...]
>
>> +
>> +	if (config->parent_slice_id && config->fixed_size) {
>> +		attr2_val |= FIELD_PREP(ATTR2_PARENT_SCID_MASK, config->parent_slice_id);
>> +		attr2_val |= ATTR2_IN_A_GROUP_MASK;
>> +	}
> This is fragile if parent_slice_id == 0, but let's say this is not an issue
> for now..

Agreed, but I don't anticipate that being an issue. I don't think any 
slice ID is/will be 0.

>
>> +
>> +	attr3_val = MAX_CAP_TO_BYTES(config->max_cap);
>> +	attr3_val /= drv_data->num_banks;
>> +	attr3_val >>= CACHE_LINE_SIZE_SHIFT;
>> +
>> +	ret = regmap_write(drv_data->bcast_regmap, attr0_cfg, attr0_val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_write(drv_data->bcast_regmap, attr1_cfg, attr1_val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_write(drv_data->bcast_regmap, attr2_cfg, attr2_val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_write(drv_data->bcast_regmap, attr3_cfg, attr3_val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	slice_offset = config->slice_id % 32;
>> +	reg_offset = (config->slice_id / 32) * 4;
>> +
>> +	wren = config->write_scid_en << slice_offset;If I'm reading the wrappers right, you should be able to drop both the
> shifting and intermediate variables with regmap_assign_bits()

I'm not so sure. I tried with regmap_assign_bits and it seems the 
correct way to use it would be roughly:

regmap_assign_bits(drv_data->bcast_regmap,
             cfg->reg_offset[LLCC_TRP_WRS_EN], BIT(config->slice_id),
             (bool)config->write_scid_en);

but the third argument is an unsigned int (the BIT(config->slice_id)). I 
tried just putting the slice_id there,
but got some bizarre results leading me to believe that's not the 
correct way to use this api. If I'm missing
something, let me know, but AFAICT, this is six one way, a half-dozen 
another.

Thanks,
Melody

>
> Looks good otherwise
>
> Konrad