Re: [PATCH v3 3/7] mfd: bcmbca-pmb: Add a shared BCMBCA PMC3 PMB transport

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mfd: bcmbca_pmc3_pmb: debugfs stall, WARN_ON, and HW read in
  regmap init
--

commit df29cb4d33202540fcf38b25c63a3a8cd862dcfb
Author: Linus Walleij <linusw@kernel.org>

mfd: bcmbca-pmb: Add a shared BCMBCA PMC3 PMB transport

This commit adds an MFD driver for the Broadcom BCMBCA PMC3 Processor
Monitor Bus (PMB) that exposes a shared uncached regmap to child devices.

> diff --git a/drivers/mfd/bcmbca_pmc3_pmb.c b/drivers/mfd/bcmbca_pmc3_pmb.c
> new file mode 100644
> index 0000000000000..8ebcbc26a502d
> --- /dev/null
> +++ b/drivers/mfd/bcmbca_pmc3_pmb.c

[ ... ]

> +static bool bcmbca_pmb_precious_reg(struct device *dev, unsigned int reg)
> +{
> +	/* Do not let debugfs probe unimplemented or firmware-owned registers. */
> +	return true;
> +}
> +
> +static struct regmap *bcmbca_pmb_regmap_init(struct bcmbca_pmb *pmb)
> +{
> +	const struct regmap_config config = {
> +		.reg_bits = 32,
> +		.val_bits = 32,
> +		.reg_stride = 1,
> +		.max_register = BCMBCA_PMB_REG(3, 255, 65535),
> +		.cache_type = REGCACHE_NONE,
> +		.precious_reg = bcmbca_pmb_precious_reg,

[Severity: Medium]
Does marking every register precious in bcmbca_pmb_precious_reg() while
setting max_register to BCMBCA_PMB_REG(3, 255, 65535) (0x03ffffff) with
reg_stride = 1 cause a 67M-iteration stall, a WARN_ON(), and an unintended
hardware read when the regmap debugfs files are read?

When the debugfs "registers" or "range" file is read,
regmap_debugfs_get_dump_start() scans all 67,108,864 addresses from 0 to
map->max_register under map->cache_lock.  Because regmap_printable() is
false for every address, map->debugfs_off_cache stays empty,
WARN_ON(list_empty(&map->debugfs_off_cache)) fires on every read, and
regmap_debugfs_get_dump_start() returns base (0).  regmap_read_debugfs()
then starts its loop at start_reg (0) without checking regmap_printable()
on the first iteration and calls regmap_read(map, 0, &val), issuing a
hardware PMB read to BCMBCA_PMB_REG(0, 0, 0).

In addition, because neither readable_reg nor writeable_reg is set, reading
the debugfs "access" file causes regmap_access_show() to format all
67,108,864 addresses into a single_open() seq_file buffer until a >1 GiB
allocation fails with -ENOMEM.

Omitting max_register and precious_reg from config (leaving max_register
at 0 without max_register_is_0) avoids creating the debugfs "registers",
"range", and "access" files while bcmbca_pmb_op() continues to validate
the bus and offset bounds.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-xg6846b-bcmbca-xrdp-reset-v3-0-d6adeb71afc9@kernel.org?part=3