Re: [PATCH for-4.22 v2 1/4] xen/arm: gic: defer host LPI allocation until after ITS init

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On 5/28/26 03:25, Mykola Kvach wrote:

Hello Mykola

> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
>
> gicv3_lpi_init_host_lpis() allocates host LPI state, including the
> host LPI lookup table, CPU notifier state and the boot CPU pending table.
> Those allocations use gicv3_its_get_memflags().
>
> ITS workarounds are discovered from gicv3_its_init(), so allocating host
> LPI state from gicv3_dist_init() can happen before the memory restrictions
> required by the ITS are known. On affected systems this can leave
> Redistributor LPI state allocated and programmed with the default memory
> policy.
>
> Move host LPI initialization after gicv3_its_init(), and only run it when
> a host ITS was found. The old call ignored the return value. Now that the
> call is made from gicv3_init(), check it and panic on failure because
> Redistributor LPI initialization relies on that state being available.
>
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
> Changes in v2:
> - Replace the v1 ITS pre-initialization hook with the less invasive
>    approach suggested during review: move the existing host LPI
>    initialization after gicv3_its_init().

Just for the context: The original review suggestion [1] was to consider 
splitting gicv3_lpi_init_host_lpis() and defer only the portions that 
depend on ITS quirks being known, specifically the allocation of the 
per-CPU pending table for the boot CPU (gicv3_lpi_allocate_pendtable), 
which is the actual consumer of gicv3_its_get_memflags(). But here, the 
whole gicv3_lpi_init_host_lpis() is moved, so the scope of the deferral 
is broader.
[1] 
https://patchew.org/Xen/cover.1774431310.git.mykola._5Fkvach@xxxxxxxx/a7732487959e777ff1de318cb28c588db69fbaa1.1774431311.git.mykola._5Fkvach@xxxxxxxx/
> - Check gicv3_lpi_init_host_lpis() and panic on failure, matching the fatal
>    nature of host LPI setup once ITS initialization succeeded.
So, this patch appears to fix two distinct issues:

- ordering issue (LPI init occurring before ITS quirks are known)
- unchecked return value from gicv3_lpi_init_host_lpis()

Should these warrant Fixes: tag(s)?


> ---
>   xen/arch/arm/gic-v3.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 17ff85ef5d..acdac22953 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -764,9 +764,6 @@ static void __init gicv3_dist_init(void)
>       type = readl_relaxed(GICD + GICD_TYPER);
>       nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
>   
> -    if ( type & GICD_TYPE_LPIS )
> -        gicv3_lpi_init_host_lpis(GICD_TYPE_ID_BITS(type));
> -
>       /* Only 1020 interrupts are supported */
>       nr_lines = min(1020U, nr_lines);
>       gicv3_info.nr_lines = nr_lines;
> @@ -1990,6 +1987,17 @@ static int __init gicv3_init(void)
>           res = gicv3_its_init();
>           if ( res )
>               panic("GICv3: ITS: initialization failed: %d\n", res);
> +
> +        /*
> +         * Host LPI allocation uses ITS-derived memory attributes, so defer it
> +         * until after gicv3_its_init() has discovered ITS workarounds.
> +         */
> +        if ( gicv3_its_host_has_its() )
This looks like a behaviour change. The condition is narrowed from "GICD 
advertises LPI support" to "host ITS is present". As a result, on a 
system where GICD_TYPE_LPIS is set but no ITS is present, LPI-specific 
variables and data structures will no longer be initialized or 
allocated. If I am not mistaken, software-generated LPIs without ITS 
involvement are currently unsupported, so this change might be safe. 
However, I think the commit message should explicitly document this 
behaviour change and explain why it is safe.

> +        {
> +            res = gicv3_lpi_init_host_lpis(intid_bits);
> +            if ( res )
> +                panic("GICv3: LPI initialization failed: %d\n", res);
> +        }
>       }
>   
>       res = gicv3_cpu_init();