Re: [PATCH RFC v2 07/10] arm_mpam: Fix mpam_dt_create_foundling_msc() to create MSC platform devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Mismatch between counted MSCs and created MSC platform devices prevents driver initialization.
--

commit bac73a6ff8544df56e037a08ea1cdb821fbb6bbc
Author: Yin Li <yin.li@oss.qualcomm.com>

arm_mpam: Fix mpam_dt_create_foundling_msc() to create MSC platform devices

This commit modifies mpam_dt_create_foundling_msc() to walk the cache's
children using for_each_child_of_node_scoped() and create a platform
device for the MSC child node, rather than for the cache node itself. It
also skips cache parents that have been marked disabled by the firmware.

> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 480e38cfa86dc..7ef3ed55b0665 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -2366,23 +2366,20 @@ static struct platform_driver mpam_msc_driver = {
>   */
>  static void mpam_dt_create_foundling_msc(void)
>  {
> -	struct platform_device *pdev;
> -	struct device_node *cache;
> -
> -	for_each_compatible_node(cache, NULL, "cache") {
> -		struct device_node *cache_device;
> -
> -		if (of_node_check_flag(cache, OF_POPULATED))
> -			continue;
> -
> -		cache_device = of_find_matching_node_and_match(cache, mpam_of_match, NULL);
> -		if (!cache_device)
> +	for_each_compatible_node_scoped(cache, NULL, "cache") {
> +		if (!of_device_is_available(cache))
>  			continue;

[Severity: High]
Does skipping the creation of platform devices for MSCs under disabled caches
cause a mismatch with fw_num_msc?

Looking at mpam_dt_count_msc(), it appears to count all available MSCs without
checking if their parent cache is actually enabled:

drivers/resctrl/mpam_devices.c:mpam_dt_count_msc() {
    ...
	for_each_compatible_node(np, NULL, "arm,mpam-msc") {
		if (of_device_is_available(np))
			count++;
	}
    ...
}

If firmware has marked a cache node as disabled but the child arm,mpam-msc
node is not explicitly disabled, this counting mismatch causes fw_num_msc to
be greater than the number of successfully probed MSCs.

As a result, would the condition in mpam_msc_drv_probe() never evaluate to
true?

drivers/resctrl/mpam_devices.c:mpam_msc_drv_probe() {
    ...
	if (atomic_add_return(1, &mpam_num_msc) == fw_num_msc)
		mpam_register_cpuhp_callbacks(mpam_discovery_cpu_online, NULL,
					      "mpam:drv_probe");
    ...
}

This could completely prevent mpam_register_cpuhp_callbacks() from being
called and leave the MPAM subsystem functionally unusable.

>  
> -		pdev = of_platform_device_create(cache, "cache", NULL);
> -		if (!pdev)
> -			pr_err_once("Failed to create MSC devices under caches\n");
> +		for_each_child_of_node_scoped(cache, child) {
> +			if (!of_match_node(mpam_of_match, child))
> +				continue;
> +			if (!of_device_is_available(child))
> +				continue;
> +			if (of_node_check_flag(child, OF_POPULATED))
> +				continue;
> +			if (!of_platform_device_create(child, NULL, NULL))
> +				pr_err("Failed to create MSC device for %pOF\n", child);
> +		}
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-mpam-resctrl-dt-knp-support-v2-0-bf6645bb2f65@oss.qualcomm.com?part=7