Re: [PATCH v2 2/2] soc: qcom: llcc-qcom: Add support for Eliza

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dynamically calculating `num_banks` creates a critical index-shifting vulnerability for `bcast_regmap` when combined with a static 4-region DT layout.
- [Medium] The LLCC_GPU slice configuration sets `.write_scid_cacheable_en = true` but omits `.write_scid_en = true`.
--

commit 33894fa3694ee52109e04ccbe94ee141b9842f00
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

soc: qcom: llcc-qcom: Add support for Eliza

This commit adds the SoC-specific configuration and compatible string for
Eliza. Eliza uses a 4-region LLCC register layout with two per-bank base
register regions and broadcast OR/AND regions.

> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 0161ceec88421..7c05cb3619b2d 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c

[ ... ]

> @@ -181,6 +181,171 @@ enum llcc_reg_offset {
> +	{
> +		.usecase_id = LLCC_GPU,
> +		.slice_id = 9,
> +		.max_cap = 896,
> +		.priority = 1,
> +		.bonus_ways = 0xfff,
> +		.write_scid_cacheable_en = true,
> +	},

The LLCC_GPU slice configures .write_scid_cacheable_en = true but omits
setting .write_scid_en = true. Without write_scid_en set, the corresponding
bit in the LLCC_TRP_WRS_EN register evaluates to 0 and is not enabled.

Without the hardware write allocator enabled for this SCID, could the
cacheable setting be rendered ineffective? Other comparable configurations
like glymur_data explicitly set both flags for the GPU slice.

[ ... ]

> @@ -4141,6 +4306,15 @@ static const u32 llcc_v6_reg_offset[] = {
>  	[LLCC_TRP_WRS_CACHEABLE_EN]	= 0x00042088,
>  };
>  
> +static const struct qcom_llcc_config eliza_cfg[] = {
> +	{
> +		.sct_data	= eliza_data,
> +		.size		= ARRAY_SIZE(eliza_data),
> +		.reg_offset	= llcc_v6_reg_offset,
> +		.edac_reg_offset = &llcc_v6_edac_reg_offset,
> +	},
> +};

Eliza uses a static 4-region Device Tree layout (llcc0, llcc1, bcast,
bcast_and). Since .num_banks is not hardcoded here, the driver will
dynamically read it from LLCC_COMMON_STATUS0.

If a defective bank is fused off on a lower-binned SKU, num_banks would be
read as 1. The probe function initializes regmaps based on num_banks:

qcom_llcc_probe() {
    ...
    for (i = 1; i < num_banks; i++) {
        ...
    }
    
    drv_data->bcast_regmap = qcom_llcc_init_mmio(pdev, i, "llcc_broadcast_base");
    ...
}

If num_banks is 1, the loop exits with i = 1. The driver will then map
index 1 from the Device Tree to bcast_regmap. Since index 1 corresponds to
llcc1_base in a static 4-region layout, could this result in broadcast
commands being incorrectly written to the llcc1 base registers, causing
memory corruption and non-functional broadcasts?

Should .num_banks = 2 be explicitly hardcoded here to prevent this index
shifting, or should the driver be updated to map resources by name instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-eliza-llcc-v2-0-27381ae833d5@oss.qualcomm.com?part=2