Re: [PATCH v9 14/14] irqchip/gic-v3-its: Preallocate VPE L1 tables

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] irqchip/gic-v3-its: order mismatch in allocate_vpe_l1_table() on ITS inherit
- [Medium] irqchip/gic-v3-its: memory leak of vpe_table_mask in allocate_vpe_l1_table()
--

commit d29e0b57c45efd854666ff7af1f84a27011e992c
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

irqchip/gic-v3-its: Preallocate VPE L1 tables

This commit removes the GFP_ATOMIC VPE L1 table page allocation from the
CPU-starting callback by using a blocking allocation on the boot CPU,
recording the table order, and preallocating pages for secondary CPUs in the
sleepable CPU hotplug prepare stage.

> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 9ad66253ab166..5628f497d389d 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c

[ ... ]

> @@ -2990,7 +3014,16 @@ static int allocate_vpe_l1_table(void)
>  
>  	pr_debug("np = %d, npg = %lld, psz = %d, epp = %d, esz = %d\n",
>  		 np, npg, psz, epp, esz);
> -	page = its_alloc_pages(GFP_ATOMIC | __GFP_ZERO, get_order(np * PAGE_SIZE));
> +	order = get_order(np * PAGE_SIZE);
> +	if (use_prealloc) {
> +		if (WARN_ON_ONCE(order != vpe_l1_prealloc_order))
> +			return -EINVAL;
> +
> +		page = gic_data_rdist()->vpe_l1_prealloc;
> +		gic_data_rdist()->vpe_l1_prealloc = NULL;
> +	} else {
> +		page = its_alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> +	}
>  	if (!page)
>  		return -ENOMEM;

[Severity: Medium]
Does this leak gic_data_rdist()->vpe_table_mask on these error returns?

Earlier in allocate_vpe_l1_table(), when inherit_vpe_l1_table_from_rd() does
not find an existing table to inherit, vpe_table_mask is allocated:

	gic_data_rdist()->vpe_table_mask = kzalloc_obj(cpumask_t, GFP_ATOMIC);
	if (!gic_data_rdist()->vpe_table_mask)
		return -ENOMEM;

If inherit_vpe_l1_table_from_its() also does not find a table to inherit and
allocate_vpe_l1_table() subsequently fails because order !=
vpe_l1_prealloc_order or page is NULL, it returns -EINVAL or -ENOMEM without
freeing gic_data_rdist()->vpe_table_mask or clearing the pointer.

> @@ -3007,6 +3040,9 @@ static int allocate_vpe_l1_table(void)
>  	val |= GICR_VPROPBASER_4_1_VALID;
>  
>  out:
> +	if (!use_prealloc)
> +		vpe_l1_prealloc_order = vpe_l1_table_order(val);

[Severity: High]
Can vpe_l1_prealloc_order mismatch the order computed for a secondary CPU when
the boot CPU inherits its VPE table from an ITS?

On the boot CPU, gic_init_bases() -> its_cpu_init(false) ->
its_cpu_init_lpis(false) -> allocate_vpe_l1_table(false) calls
inherit_vpe_l1_table_from_its():

	val = inherit_vpe_l1_table_from_its();
	if (val & GICR_VPROPBASER_4_1_VALID)
		goto out;

When the boot CPU redistributor shares a VPE table with an ITS in its affinity
domain, this jumps to out: without probing GICR_VPROPBASER, and
vpe_l1_prealloc_order is computed from the inherited GITS_BASER2 value.

However, the ITS VPE table is sized by its_parse_indirect_baser(), which only
switches to a two-level indirect table when the flat table exceeds two GIC
pages and GITS_BASER_INDIRECT is supported:

	/* No need to enable Indirection if memory requirement < (psz*2)bytes */
	if ((esz << ids) > (psz * 2)) {

By contrast, when a secondary CPU in another affinity domain cannot inherit
from a redistributor or an ITS, allocate_vpe_l1_table(true) probes
GICR_VPROPBASER on the redistributor and enables indirection whenever the flat
table exceeds a single GIC page:

	if (epp < ITS_MAX_VPEID) {
		int nl2;

		val |= GICR_VPROPBASER_4_1_INDIRECT;

For example, when the flat table is 2 * psz (or when GITS_BASER2 uses a flat
table or a different probed page size than GICR_VPROPBASER), the inherited ITS
table order is get_order(2 * psz) while the secondary redistributor L1 table
order is get_order(psz).

When that secondary CPU comes online, wouldn't allocate_vpe_l1_table(true) hit
WARN_ON_ONCE(order != vpe_l1_prealloc_order), return -EINVAL, and cause
its_cpu_init_lpis() to disable GICv4 support?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104164555.3555725-1-aneesh.kumar@kernel.org?part=14