Re: [PATCH v3 8/8] riscv_cbqri: Add CBQRI capacity allocation platform driver

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Drew,

On 6/28/26 2:18 PM, Drew Fustini wrote:

...> diff --git a/drivers/resctrl/cbqri_capacity.c b/drivers/resctrl/cbqri_capacity.c
> new file mode 100644
> index 000000000000..2172432eb328
> --- /dev/null
> +++ b/drivers/resctrl/cbqri_capacity.c

...

> +static int cbqri_capacity_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cbqri_controller_info info = {};
> +	struct device_node *cache_np;
> +	cpumask_var_t cpu_mask;
> +	struct resource *res;
> +	u32 rcid_count, cache_level;
> +	int cache_id, cpu, ret;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -EINVAL;
> +
> +	ret = of_property_read_u32(dev->of_node, "riscv,cbqri-rcid", &rcid_count);
> +	if (ret) {
> +		dev_err(dev, "missing riscv,cbqri-rcid\n");
> +		return ret;
> +	}
> +
> +	cache_np = of_parse_phandle(dev->of_node, "riscv,cbqri-cache", 0);
> +	if (!cache_np) {
> +		dev_err(dev, "missing riscv,cbqri-cache phandle\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(cache_np, "cache-level", &cache_level);
> +	if (ret) {
> +		dev_err(dev, "%pOF: missing cache-level\n", cache_np);
> +		goto out_put;
> +	}
> +
> +	if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL)) {
> +		ret = -ENOMEM;
> +		goto out_put;
> +	}
> +
> +	/*
> +	 * Associate the controller with its cache instance via
> +	 * cacheinfo. The matching cache provides the cache id and the
> +	 * set of harts that share the cache.
> +	 */
> +	cache_id = -1;
> +	cpus_read_lock();
> +	for_each_online_cpu(cpu) {
> +		struct cacheinfo *ci = get_cpu_cacheinfo_level(cpu, cache_level);
> +
> +		if (ci && ci->fw_token == cache_np) {
> +			cache_id = ci->id;
> +			cpumask_copy(cpu_mask, &ci->shared_cpu_map);

The way I understand cacheinfo::shared_cpu_map is that it only contains the online
CPUs that share the cache with this CPU and if the CPU is offline then shared_cpu_map
only contains the CPU self.

It is thus not clear to me that this handles all the possible CPU online vs offline
scenarios. For example, if all or some CPUs of a domain are offline during cbqri_capacity_probe()
and then come online later. It is not clear to me whether cbqri_controller_info::cache_id,
cbqri_controller::cache_controller::cache_id, or cbqri_controller::cache_controller::cpu_mask
are needed. Could the cache ID associated with a CPU at the time it comes online to dynamically
associate it with the resctrl domain that is indexed by the cache ID? This may simplify a couple
of flows.

> +			break;
> +		}
> +	}
> +	cpus_read_unlock();
> +
> +	if (cache_id < 0) {
> +		dev_err(dev, "%pOF: no online hart reports an L%u cache for this node\n",
> +			cache_np, cache_level);
> +		ret = -ENODEV;
> +		goto out_free;
> +	}
> +
> +	info.type = CBQRI_CONTROLLER_TYPE_CAPACITY;
> +	info.addr = res->start;
> +	info.size = resource_size(res);
> +	info.rcid_count = rcid_count;
> +	info.cache_id = cache_id;
> +
> +	ret = riscv_cbqri_register_cc_dt(&info, cache_level, cpu_mask);
> +	if (ret) {
> +		dev_err(dev, "failed to register capacity controller: %d\n", ret);
> +		goto out_free;
> +	}
> +
> +	dev_info(dev, "registered L%u capacity controller at %pa (cache_id=%d, rcid=%u)\n",
> +		 cache_level, &info.addr, cache_id, rcid_count);
> +
> +out_free:
> +	free_cpumask_var(cpu_mask);
> +out_put:
> +	of_node_put(cache_np);
> +	return ret;
> +}
Reinette


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv