Re: [PATCH 4/4] xen/arm: its: pre-initialize ITS quirks before LPI setup

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On 3/25/26 12:38, Mykola Kvach wrote:

Hello Mykola

> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
>
> In the current initialization flow, gicv3_init() calls gicv3_dist_init()
> before gicv3_its_init().
>
> When LPIs are supported, gicv3_dist_init() calls
> gicv3_lpi_init_host_lpis(), which initializes host LPI state and allocates
> the boot CPU pending table before ITS quirk flags are computed. Non-boot
> CPUs allocate their pending tables later from the CPU_UP_PREPARE notifier,
> while redistributor LPI programming happens separately in
> gicv3_lpi_init_rdist().
>
> This means the boot CPU LPI setup can observe default ITS memory attributes
> before dma-noncoherent and other ITS quirks are applied.
>
> Introduce gicv3_its_preinit() and call it before gicv3_dist_init(). This
> keeps the actual ITS hardware initialization in gicv3_its_init(), but moves
> ITS discovery, quirk validation and quirk flag setup early enough for the
> subsequent LPI initialization to use the correct attributes.

Have you considered an alternative approach that might be less invasive? 
I am thinking of something the other way around: perhaps we could 
allocate the LPI pending table for the boot CPU later.
Would a diff below work?


---
 xen/arch/arm/gic-v3-lpi.c             | 9 +++++++++
 xen/arch/arm/gic-v3.c                 | 2 ++
 xen/arch/arm/include/asm/gic_v3_its.h | 6 ++++++
 3 files changed, 17 insertions(+)

diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
index 9ee338edc2..61cc45d347 100644
--- a/xen/arch/arm/gic-v3-lpi.c
+++ b/xen/arch/arm/gic-v3-lpi.c
@@ -450,6 +450,15 @@ int gicv3_lpi_init_host_lpis(unsigned int 
host_lpi_bits)
     printk("GICv3: using at most %lu LPIs on the host.\n", 
MAX_NR_HOST_LPIS);
+    return rc;
+}
+
+int gicv3_lpi_post_init_host_lpis(void)
+{
+    int rc;
+
+    ASSERT(smp_processor_id() == 0);
+
     /* Register the CPU notifier and allocate memory for the boot CPU */
     register_cpu_notifier(&cpu_nfb);
     rc = gicv3_lpi_allocate_pendtable(smp_processor_id());
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 7f365cdbe9..8b9059c5c9 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1981,6 +1981,8 @@ static int __init gicv3_init(void)
         res = gicv3_its_init();
         if ( res )
             panic("GICv3: ITS: initialization failed: %d\n", res);
+
+        gicv3_lpi_post_init_host_lpis();
     }

     res = gicv3_cpu_init();
diff --git a/xen/arch/arm/include/asm/gic_v3_its.h 
b/xen/arch/arm/include/asm/gic_v3_its.h
index fc5a84892c..288cc1d42f 100644
--- a/xen/arch/arm/include/asm/gic_v3_its.h
+++ b/xen/arch/arm/include/asm/gic_v3_its.h
@@ -156,6 +156,7 @@ int gicv3_lpi_init_rdist(void __iomem * rdist_base);

 /* Initialize the host structures for LPIs and the host ITSes. */
 int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits);
+int gicv3_lpi_post_init_host_lpis(void);
 int gicv3_its_init(void);

 /* Store the physical address and ID for each redistributor as read 
from DT. */
@@ -245,6 +246,11 @@ static inline int gicv3_lpi_init_host_lpis(unsigned 
int host_lpi_bits)
     return 0;
 }

+static inline int gicv3_lpi_post_init_host_lpis(void)
+{
+    return 0;
+}
+
 static inline int gicv3_its_init(void)
 {
     return 0;
--
2.34.1


> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
> TODO: Think about separating Redistributor/LPI attributes from ITS.
> ---
>   xen/arch/arm/gic-v3-its.c             | 36 +++++++++++++++++----------
>   xen/arch/arm/gic-v3.c                 |  7 ++++++
>   xen/arch/arm/include/asm/gic_v3_its.h |  5 ++++
>   3 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index ee432088cd..0251d91630 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -63,6 +63,7 @@ struct its_quirk {
>       uint32_t flags;
>   };
>   
> +/* TODO: Separate Redistributor/LPI attributes from ITS quirks. */
>   static uint32_t __ro_after_init its_quirk_flags;
>   
>   static bool gicv3_its_match_quirk_gen4(const struct host_its *hw_its)
> @@ -148,9 +149,15 @@ static uint32_t gicv3_its_collect_quirks(const struct 
> host_its *hw_its,
>       return flags;
>   }
>   
> -static void gicv3_its_enable_quirks(struct host_its *hw_its)
> +static void gicv3_its_enable_quirks(void)
>   {
>       const struct its_quirk *quirk;
> +    const struct host_its *hw_its;
> +
> +    if ( list_empty(&host_its_list) )
> +        return;
> +
> +    hw_its = list_first_entry(&host_its_list, struct host_its, entry);
>   
>       its_quirk_flags = gicv3_its_collect_quirks(hw_its, &quirk);
>   
> @@ -603,16 +610,10 @@ static int gicv3_its_init_single_its(struct host_its *hw_its)
>       uint64_t reg;
>       int i, ret;
>   
> -    hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
> -    if ( !hw_its->its_base )
> -        return -ENOMEM;
> -
>       ret = gicv3_disable_its(hw_its);
>       if ( ret )
>           return ret;
>   
> -    gicv3_its_enable_quirks(hw_its);
> -
>       reg = readq_relaxed(hw_its->its_base + GITS_TYPER);
>       hw_its->devid_bits = GITS_TYPER_DEVICE_ID_BITS(reg);
>       hw_its->evid_bits = GITS_TYPER_EVENT_ID_BITS(reg);
> @@ -1161,6 +1162,11 @@ static void add_to_host_its_list(paddr_t addr, paddr_t 
> size,
>       its_data->size = size;
>       its_data->dt_node = node;
>   
> +    its_data->its_base = ioremap_nocache(its_data->addr, its_data->size);
> +    if ( !its_data->its_base )
> +        panic("GICv3: Cannot map ITS frame: 0x%lx, 0x%lx\n",
> +            its_data->addr, its_data->size);
> +
>       printk("GICv3: Found ITS @0x%lx\n", addr);
>   
>       list_add_tail(&its_data->entry, &host_its_list);
> @@ -1238,16 +1244,22 @@ static void gicv3_its_acpi_init(void)
>   
>   #endif
>   
> -int gicv3_its_init(void)
> +void __init gicv3_its_preinit(void)
>   {
> -    struct host_its *hw_its;
> -    int ret;
> -
>       if ( acpi_disabled )
>           gicv3_its_dt_init(dt_interrupt_controller);
>       else
>           gicv3_its_acpi_init();
>   
> +    gicv3_its_validate_quirks();
> +    gicv3_its_enable_quirks();
> +}
> +
> +int gicv3_its_init(void)
> +{
> +    struct host_its *hw_its;
> +    int ret;
> +
>       list_for_each_entry(hw_its, &host_its_list, entry)
>       {
>           ret = gicv3_its_init_single_its(hw_its);
> @@ -1255,8 +1267,6 @@ int gicv3_its_init(void)
>               return ret;
>       }
>   
> -    gicv3_its_validate_quirks();
> -
>       return 0;
>   }
>   
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index bc07f97c16..6e44d23d64 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1974,6 +1974,13 @@ static int __init gicv3_init(void)
>   
>       spin_lock(&gicv3.lock);
>   
> +    if ( gic_dist_supports_lpis() )
> +        /*
> +         * Apply ITS quirks before gicv3_dist_init() sets up host LPIs,
> +         * so pending tables use the correct ITS memory attributes.
> +         */
> +        gicv3_its_preinit();
> +
>       gicv3_dist_init();
>   
>       if ( gic_dist_supports_lpis() )
> diff --git a/xen/arch/arm/include/asm/gic_v3_its.h 
> b/xen/arch/arm/include/asm/gic_v3_its.h
> index fc5a84892c..e1d7522ea5 100644
> --- a/xen/arch/arm/include/asm/gic_v3_its.h
> +++ b/xen/arch/arm/include/asm/gic_v3_its.h
> @@ -156,6 +156,7 @@ int gicv3_lpi_init_rdist(void __iomem * rdist_base);
>   
>   /* Initialize the host structures for LPIs and the host ITSes. */
>   int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits);
> +void gicv3_its_preinit(void);
>   int gicv3_its_init(void);
>   
>   /* Store the physical address and ID for each redistributor as read from DT. */
> @@ -219,6 +220,10 @@ static inline int gicv3_its_deny_access(struct domain *d)
>       return 0;
>   }
>   
> +static inline void gicv3_its_preinit(void)
> +{
> +}
> +
>   static inline bool gicv3_its_host_has_its(void)
>   {
>       return false;