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

Steven Price <[email protected]>
Newsgroups org.kernel.vger.linux-media,org.freedesktop.lists.dri-devel,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
> ---
>  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;
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.