Re: [PATCH v2 2/4] irqchip/gic-v3-its: Allocate VPE tables from sleepable context

[Steven Price]

On 20/08/2026 16:00, Steven Price wrote:
> The VPE L1 table is allocated from the CPU-starting hotplug state,
> where interrupts are disabled. Although the page allocation uses
> GFP_ATOMIC, its_alloc_pages() subsequently calls
> set_memory_decrypted(), which can sleep while splitting the arm64
> linear map.
> 
> Move the allocation to the existing CPU-online callback, which runs in
> sleepable context, and use GFP_KERNEL for both allocations performed
> there. Register the callback even without EFI, since it is now also
> responsible for VPE table allocation.

Ok, Sashiko pointed out this is bunk:

> Does moving this allocation to the CPUHP_AP_ONLINE_DYN callback expose
> uninitialized GICv4.1 redistributor state to concurrent VPE mappings?
> 
> The CPUHP_AP_ONLINE_DYN state runs for its_cpu_memreserve_lpi() in the hotplug
> thread asynchronously after the CPU has already been marked online in
> cpu_online_mask.
> 
> Could a concurrent process changing IRQ affinities (like irqbalance) observe
> the CPU in cpu_online_mask and route a virtual interrupt to it via
> its_vpe_set_affinity() before the hotplug thread has executed this table
> allocation?
> 
> If a VMAPP command executes before allocate_vpe_l1_table() programs the
> GICR_VPROPBASER, it appears the hardware could use stale or uninitialized
> physical addresses, leading to unpredictable behavior or dropped interrupts.

Please ignore this patch. I thought this was a bit too simple to fix :(
I guess some sort of preallocation might be the solution.

Thanks,
Steve

> Fixes: b08e2f42e86b ("irqchip/gic-v3-its: Share ITS tables with a non-trusted hypervisor")
> Signed-off-by: Steven Price <steven.price@xxxxxxx>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index a055837832bc..71515dbff9ec 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2932,7 +2932,7 @@ static int allocate_vpe_l1_table(void)
>  	if (val & GICR_VPROPBASER_4_1_VALID)
>  		goto out;
>  
> -	gic_data_rdist()->vpe_table_mask = kzalloc_obj(cpumask_t, GFP_ATOMIC);
> +	gic_data_rdist()->vpe_table_mask = kzalloc_obj(cpumask_t, GFP_KERNEL);
>  	if (!gic_data_rdist()->vpe_table_mask)
>  		return -ENOMEM;
>  
> @@ -2999,7 +2999,7 @@ 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));
> +	page = its_alloc_pages(GFP_KERNEL | __GFP_ZERO, get_order(np * PAGE_SIZE));
>  	if (!page)
>  		return -ENOMEM;
>  
> @@ -3268,16 +3268,6 @@ static void its_cpu_init_lpis(void)
>  		val = its_clear_vpend_valid(vlpi_base, 0, 0);
>  	}
>  
> -	if (allocate_vpe_l1_table()) {
> -		/*
> -		 * If the allocation has failed, we're in massive trouble.
> -		 * Disable direct injection, and pray that no VM was
> -		 * already running...
> -		 */
> -		gic_rdists->has_rvpeid = false;
> -		gic_rdists->has_vlpis = false;
> -	}
> -
>  	/* Make sure the GIC has seen the above */
>  	dsb(sy);
>  	gic_data_rdist()->flags |= RD_LOCAL_LPI_ENABLED;
> @@ -5452,6 +5442,19 @@ static int its_cpu_memreserve_lpi(unsigned int cpu)
>  	if (gic_data_rdist()->flags & RD_LOCAL_MEMRESERVE_DONE)
>  		return 0;
>  
> +	if (allocate_vpe_l1_table()) {
> +		/*
> +		 * If the allocation has failed, we're in massive trouble.
> +		 * Disable direct injection, and pray that no VM was
> +		 * already running...
> +		 */
> +		gic_rdists->has_rvpeid = false;
> +		gic_rdists->has_vlpis = false;
> +	}
> +
> +	if (!efi_enabled(EFI_CONFIG_TABLES))
> +		goto out;
> +
>  	pend_page = gic_data_rdist()->pend_page;
>  	if (WARN_ON(!pend_page)) {
>  		ret = -ENOMEM;
> @@ -5793,9 +5796,6 @@ int __init its_lpi_memreserve_init(void)
>  {
>  	int state;
>  
> -	if (!efi_enabled(EFI_CONFIG_TABLES))
> -		return 0;
> -
>  	if (list_empty(&its_nodes))
>  		return 0;
>