Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Moving VPE L1 table allocation to a late CPU hotplug state exposes 
uninitialized GICv4.1 redistributor state to concurrent VPE mappings.
--

--- Patch [2]: [PATCH v2 2/4] irqchip/gic-v3-its: Allocate VPE tables from 
sleepable context ---
commit 7ed963cf27a623f2c21ad61ef0162e3eddac65ae
Author: Steven Price <[email protected]>

irqchip/gic-v3-its: Allocate VPE tables from sleepable context

This patch moves the VPE L1 table allocation from the early CPU-starting
hotplug state to the later CPU-online callback. This permits the use of
GFP_KERNEL allocations, as its_alloc_pages() can sleep. The CPU-online
callback is now registered unconditionally to handle this setup.

> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> b/drivers/irqchip/irq-gic-v3-its.c
> index bfd58097261a7..6452888b34fcb 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
[ ... ]
> @@ -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;
[ ... ]
> @@ -5454,6 +5444,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;
> +     }
> +

[Severity: High]
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.

>       if (!efi_enabled(EFI_CONFIG_TABLES))
>               goto out;
>

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to