* [PATCH v3 0/4] xen/arm: gicv3: defer host LPI init and split ITS/LPI quirk scopes
@ 2026-09-22 15:58 Mykola Kvach
2026-09-22 15:58 ` [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes Mykola Kvach
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Mykola Kvach @ 2026-09-22 15:58 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk
Hi all,
This series reworks ITS and host LPI initialization, separates
ITS-private memory restrictions from host LPI/Redistributor
restrictions, and adds support for the DT dma-noncoherent property.
Patch 1 splits ITS initialization into preparation and activation
phases. The preparation pass maps and disables each host ITS and
collects its quirks. Once all host ITSes have been prepared, host LPI
state is initialized, including the boot CPU pending table. Only then
are ITS tables and command queues allocated and programmed, and the
ITSes enabled. This sequence is coordinated within gicv3_its_init().
The subsequent gicv3_cpu_init() sequence remains unchanged: it programs
the Redistributor LPI table registers, enables LPIs, and submits the
first MAPC and SYNC commands.
Patch 2 replaces the global ITS quirk state with per-ITS flags and a
separate host LPI policy. Per-ITS flags govern ITS table, command queue,
and ITT allocations, and the memory attributes programmed into
GITS_CBASER and GITS_BASER<n>. Host LPI flags govern property and
pending table allocations and the attributes programmed into
GICR_PROPBASER and GICR_PENDBASER.
An ITS-discovered quirk contributes to host LPI policy only through its
explicit lpi_flags. Per-ITS flags are not implicitly aggregated into
that policy, and different ITSes no longer have to share the same quirk
state.
Patch 3 replaces the fixed IIDR/mask fields in the quirk table with a
match(hw_its, data) callback and an opaque data pointer. A reusable IIDR
matcher is combined with platform checks for the R-Car Gen4 entry.
First-match semantics are preserved.
The R-Car Gen4 entry matches only DT-discovered ITSes with IIDR
0x0201743b on machines compatible with renesas,r8a779f0 or
renesas,r8a779g0. ACPI-discovered ITSes and other platforms with the
same IIDR no longer match this entry. For matching systems, the
non-cacheable/non-shareable attributes and 32-bit allocation
restrictions are retained in both the ITS and host LPI scopes.
Patch 4 handles dma-noncoherent independently of platform quirk
matching. A property on an ITS subnode affects only that ITS. A property
on the top-level GIC node affects only host LPI/Redistributor policy.
The property is not inherited between the nodes. It selects
non-cacheable/non-shareable memory attributes; it does not impose a
32-bit allocation restriction.
---
Changes since v2
* Split ITS initialization into preparation and activation phases,
with host LPI initialization between them inside gicv3_its_init().
* Collected quirk flags during ITS preparation. Kept the ITS init-only
annotations in the initialization patch.
* Documented the intentional narrowing of the R-Car Gen4 match,
including the exclusion of ACPI-discovered ITSes and other platforms
with the same IIDR.
* Added the non-debug NULL-data guard to the IIDR matcher and placed
the matcher data and quirk table in init-only read-only sections.
* Dropped the redundant aggregate host-LPI-flags log message
---
Changes since v1
* Reordered the series so that the minimal host LPI initialization ordering fix
is first. Patch 1 is intended for 4.22.
* Dropped the v1 ITS pre-initialization hook.
* Moved the existing gicv3_lpi_init_host_lpis() call after gicv3_its_init()
instead, so host LPI state is allocated after ITS workaround discovery.
* Checked the return value from gicv3_lpi_init_host_lpis() and made failure
fatal once the ITS/LPI path is enabled.
* Replaced the old single global ITS quirk state with separate per-ITS and
host LPI quirk scopes.
* Removed the implicit aggregation of all per-ITS quirks into the host LPI
policy. Host LPI effects are now expressed explicitly with lpi_flags.
* Kept per-ITS flags for ITS-private allocations:
- GITS_CBASER;
- GITS_BASER<n>;
- ITT memory.
* Kept host LPI flags for Redistributor/LPI state:
- GICR_PROPBASER;
- GICR_PENDBASER.
* Refactored ITS quirk matching from fixed IIDR/mask fields to a generic
match(hw_its, data) callback plus opaque data.
* Kept first-match semantics explicit. More specific entries must be listed
before broader IIDR-only entries.
* Added a reusable IIDR matcher and used it after checking the Renesas
machine compatibles for the R-Car Gen4 quirk.
* Split dma-noncoherent handling by DT node scope:
- ITS subnode dma-noncoherent affects only the matching ITS;
- top-level GIC dma-noncoherent affects only the host LPI/Redistributor
policy.
* Dropped the Orange Pi 5 / RK3588-specific quirk patch from v1. The
non-coherent GIC integration is now handled through DT dma-noncoherent
properties instead of a Xen-side platform quirk.
Mykola Kvach (4):
xen/arm: its: initialize host LPI state before activating ITSes
xen/arm: its: separate ITS and host LPI quirk scopes
xen/arm: its: refactor ITS quirk matching
xen/arm: its: handle dma-noncoherent on GIC and ITS nodes
xen/arch/arm/gic-v3-its.c | 203 +++++++++++++++++---------
xen/arch/arm/gic-v3-lpi.c | 65 +++++++--
xen/arch/arm/gic-v3.c | 15 +-
xen/arch/arm/include/asm/gic_v3_its.h | 22 ++-
4 files changed, 210 insertions(+), 95 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes
2026-09-22 15:58 [PATCH v3 0/4] xen/arm: gicv3: defer host LPI init and split ITS/LPI quirk scopes Mykola Kvach
@ 2026-09-22 15:58 ` Mykola Kvach
2026-09-22 23:56 ` Volodymyr Babchuk
2026-09-22 15:58 ` [PATCH v3 2/4] xen/arm: its: separate ITS and host LPI quirk scopes Mykola Kvach
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Mykola Kvach @ 2026-09-22 15:58 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk
The boot CPU pending table must use the memory attributes selected by
ITS quirks. gicv3_lpi_init_host_lpis() is therefore called after
gicv3_its_init(). However, gicv3_its_init() also programs and enables
each ITS before host LPI state is allocated. No ITS commands are
submitted at that point, but this ordering relies on that implementation
detail.
Split per-ITS initialization into preparation and activation phases.
First map and disable every ITS and collect its quirks. Then initialize
host LPI state. Only after that, allocate and program the ITS tables and
command queue, and enable each ITS.
The subsequent gicv3_cpu_init() sequence remains unchanged: it programs
the Redistributor LPI tables, enables LPIs, and then submits the first
MAPC and SYNC commands.
Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
Changes in v3:
- New patch implementing the post-4.22 initialization order discussed
during review of the ordering fix.
Link: https://patchew.org/Xen/341edd8de63dcd84ccc6e7b6c03e9e8fc7105184.1781847061.git.mykola._5Fkvach@epam.com/
---
xen/arch/arm/gic-v3-its.c | 32 ++++++++++++++++++++++-----
xen/arch/arm/gic-v3.c | 15 ++-----------
xen/arch/arm/include/asm/gic_v3_its.h | 4 ++--
3 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 325835b0ad..972825bf06 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -11,6 +11,7 @@
#include <xen/lib.h>
#include <xen/delay.h>
#include <xen/iocap.h>
+#include <xen/init.h>
#include <xen/libfdt/libfdt.h>
#include <xen/mm.h>
#include <xen/rbtree.h>
@@ -549,10 +550,9 @@ static int gicv3_disable_its(struct host_its *hw_its)
return -ETIMEDOUT;
}
-static int gicv3_its_init_single_its(struct host_its *hw_its)
+static int __init gicv3_its_prepare_single_its(struct host_its *hw_its)
{
- uint64_t reg;
- int i, ret;
+ int ret;
hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
if ( !hw_its->its_base )
@@ -564,6 +564,14 @@ static int gicv3_its_init_single_its(struct host_its *hw_its)
gicv3_its_enable_quirks(hw_its);
+ return 0;
+}
+
+static int __init gicv3_its_init_single_its(struct host_its *hw_its)
+{
+ uint64_t reg;
+ int i, ret;
+
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);
@@ -1189,7 +1197,7 @@ static void gicv3_its_acpi_init(void)
#endif
-int gicv3_its_init(void)
+int __init gicv3_its_init(unsigned int host_lpi_bits)
{
struct host_its *hw_its;
int ret;
@@ -1201,13 +1209,27 @@ int gicv3_its_init(void)
list_for_each_entry(hw_its, &host_its_list, entry)
{
- ret = gicv3_its_init_single_its(hw_its);
+ ret = gicv3_its_prepare_single_its(hw_its);
if ( ret )
return ret;
}
gicv3_its_validate_quirks();
+ if ( list_empty(&host_its_list) )
+ return 0;
+
+ ret = gicv3_lpi_init_host_lpis(host_lpi_bits);
+ if ( ret )
+ return ret;
+
+ list_for_each_entry(hw_its, &host_its_list, entry)
+ {
+ ret = gicv3_its_init_single_its(hw_its);
+ if ( ret )
+ return ret;
+ }
+
return 0;
}
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index acdac22953..6bc3e313be 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1984,20 +1984,9 @@ static int __init gicv3_init(void)
if ( gic_dist_supports_lpis() )
{
- res = gicv3_its_init();
+ res = gicv3_its_init(intid_bits);
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() )
- {
- res = gicv3_lpi_init_host_lpis(intid_bits);
- if ( res )
- panic("GICv3: LPI initialization failed: %d\n", res);
- }
+ panic("GICv3: ITS/LPI initialization failed: %d\n", res);
}
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 02c083f210..e1322b94f8 100644
--- a/xen/arch/arm/include/asm/gic_v3_its.h
+++ b/xen/arch/arm/include/asm/gic_v3_its.h
@@ -156,7 +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_its_init(void);
+int gicv3_its_init(unsigned int host_lpi_bits);
/* Store the physical address and ID for each redistributor as read from DT. */
void gicv3_set_redist_address(paddr_t address, unsigned int redist_id);
@@ -245,7 +245,7 @@ static inline int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
return 0;
}
-static inline int gicv3_its_init(void)
+static inline int gicv3_its_init(unsigned int host_lpi_bits)
{
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 2/4] xen/arm: its: separate ITS and host LPI quirk scopes
2026-09-22 15:58 [PATCH v3 0/4] xen/arm: gicv3: defer host LPI init and split ITS/LPI quirk scopes Mykola Kvach
2026-09-22 15:58 ` [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes Mykola Kvach
@ 2026-09-22 15:58 ` Mykola Kvach
2026-09-23 0:13 ` Volodymyr Babchuk
2026-09-22 15:58 ` [PATCH v3 3/4] xen/arm: its: refactor ITS quirk matching Mykola Kvach
2026-09-22 15:58 ` [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes Mykola Kvach
3 siblings, 1 reply; 16+ messages in thread
From: Mykola Kvach @ 2026-09-22 15:58 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk
ITS quirks can impose restrictions on memory accessed by the ITS itself
and on shared host LPI/Redistributor state. These scopes are not
identical, so a single global ITS quirk state makes the host LPI policy
depend implicitly on the quirks seen while initializing host ITSes.
Add per-ITS quirk_flags to struct host_its and keep a separate
host_lpi_flags state in the LPI code. The quirk table now records the
ITS-private and host LPI scopes explicitly through its_flags and
lpi_flags. The R-Car Gen4 quirk applies the same memory restrictions to
both scopes, preserving the existing R-Car memory policy without relying
on an implicit aggregation step.
This also removes the assumption that all host ITSes must expose the
same quirk state. ITS-derived host LPI restrictions are accumulated only
from quirk entries that explicitly set lpi_flags.
Use per-ITS quirk_flags for GITS_CBASER, GITS_BASER<n>, and ITT
allocations. Use host_lpi_flags when allocating the property and pending
tables and when programming GICR_PROPBASER and GICR_PENDBASER.
Memory-related quirk bits are named GICV3_QUIRK_MEM_* and are translated
by shared gicv3_mem_get_*() helpers.
Collect both scopes during the ITS preparation pass, before host LPI
state is allocated. Keep init-only annotations and includes in the
implementation files rather than the public header.
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
Changes in v3:
- Collect quirk flags during the ITS preparation pass added earlier.
- Keep init-only annotations and includes in implementation files.
Changes in v2:
- Replace v1's single global ITS quirk flags and same-quirk validation
with explicit per-ITS and host LPI quirk scopes.
- Drop the v1 ITS pre-initialization approach from this patch. In v2,
host LPI allocation was moved after ITS initialization by a separate
patch.
- Drop DT dma-noncoherent handling from this patch; firmware-provided
non-coherency is handled separately with explicit ITS-node and
GIC-node scopes.
- Keep host_lpi_flags owned by gic-v3-lpi.c and update it only with quirk
flags that explicitly apply to host LPI/Redistributor state.
- Rename the current memory-related bits to GICV3_QUIRK_MEM_* and use
shared gicv3_mem_get_*() helpers for register attributes and allocation
flags.
---
xen/arch/arm/gic-v3-its.c | 111 ++++++++++++--------------
xen/arch/arm/gic-v3-lpi.c | 45 +++++++++--
xen/arch/arm/include/asm/gic_v3_its.h | 18 +++--
3 files changed, 100 insertions(+), 74 deletions(-)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 972825bf06..454fd0e0ef 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -52,43 +52,40 @@ struct its_device {
struct pending_irq *pend_irqs; /* One struct per event */
};
-/*
- * It is unlikely that a platform implements ITSes with different quirks,
- * so assume they all share the same.
- */
struct its_quirk {
const char *desc;
- bool (*init)(struct host_its *hw_its);
uint32_t iidr;
uint32_t mask;
+ uint32_t its_flags;
+ /*
+ * lpi_flags are ORed into the global host LPI policy and must only
+ * contain additive restrictions. Non-additive LPI quirks need explicit
+ * handling.
+ */
+ uint32_t lpi_flags;
};
-static uint32_t __ro_after_init its_quirk_flags;
-
-static bool gicv3_its_enable_quirk_gen4(struct host_its *hw_its)
-{
- its_quirk_flags |= HOST_ITS_WORKAROUND_NC_NS |
- HOST_ITS_WORKAROUND_32BIT_ADDR;
-
- return true;
-}
-
static const struct its_quirk its_quirks[] = {
{
.desc = "R-Car Gen4",
.iidr = 0x0201743b,
.mask = 0xffffffffU,
- .init = gicv3_its_enable_quirk_gen4,
+ .its_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
+ .lpi_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
},
{
/* Sentinel. */
}
};
-static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
+static const struct its_quirk *__init gicv3_its_find_quirk(uint32_t iidr)
{
const struct its_quirk *quirks = its_quirks;
+ /*
+ * The first matching quirk wins. More specific quirks must be listed
+ * before broader IIDR-only entries.
+ */
for ( ; quirks->desc; quirks++ )
{
if ( quirks->iidr == (quirks->mask & iidr) )
@@ -98,53 +95,38 @@ static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
return NULL;
}
-static void gicv3_its_enable_quirks(struct host_its *hw_its)
+static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
{
uint32_t iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
const struct its_quirk *quirk = gicv3_its_find_quirk(iidr);
- if ( quirk && quirk->init(hw_its) )
- printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
-}
-
-static void gicv3_its_validate_quirks(void)
-{
- const struct its_quirk *quirk = NULL, *prev = NULL;
- 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);
- prev = gicv3_its_find_quirk(readl_relaxed(hw_its->its_base + GITS_IIDR));
-
- list_for_each_entry(hw_its, &host_its_list, entry)
+ if ( quirk )
{
- quirk = gicv3_its_find_quirk(readl_relaxed(hw_its->its_base + GITS_IIDR));
- BUG_ON(quirk != prev);
- prev = quirk;
+ hw_its->quirk_flags |= quirk->its_flags;
+ gicv3_lpi_update_host_flags(quirk->lpi_flags);
+ printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
}
}
-uint64_t gicv3_its_get_cacheability(void)
+uint64_t gicv3_mem_get_cacheability(uint32_t flags)
{
- if ( its_quirk_flags & HOST_ITS_WORKAROUND_NC_NS )
+ if ( flags & GICV3_QUIRK_MEM_NC_NS )
return GIC_BASER_CACHE_nC;
return GIC_BASER_CACHE_RaWaWb;
}
-uint64_t gicv3_its_get_shareability(void)
+uint64_t gicv3_mem_get_shareability(uint32_t flags)
{
- if ( its_quirk_flags & HOST_ITS_WORKAROUND_NC_NS )
+ if ( flags & GICV3_QUIRK_MEM_NC_NS )
return GIC_BASER_NonShareable;
return GIC_BASER_InnerShareable;
}
-unsigned int gicv3_its_get_memflags(void)
+unsigned int gicv3_mem_get_alloc_flags(uint32_t flags)
{
- if ( its_quirk_flags & HOST_ITS_WORKAROUND_32BIT_ADDR )
+ if ( flags & GICV3_QUIRK_MEM_32BIT_ADDR )
return MEMF_bits(32);
return 0;
@@ -391,13 +373,17 @@ static void *its_map_cbaser(struct host_its *its)
uint64_t reg;
unsigned int order;
void *buffer;
+ uint64_t cacheability = gicv3_mem_get_cacheability(its->quirk_flags);
+ uint64_t shareability = gicv3_mem_get_shareability(its->quirk_flags);
+ unsigned int memflags = gicv3_mem_get_alloc_flags(its->quirk_flags);
- reg = gicv3_its_get_shareability() << GITS_BASER_SHAREABILITY_SHIFT;
- reg |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
- reg |= gicv3_its_get_cacheability() << GITS_BASER_INNER_CACHEABILITY_SHIFT;
+ reg = MASK_INSR(shareability, GITS_BASER_SHAREABILITY_MASK);
+ reg |= MASK_INSR(GIC_BASER_CACHE_SameAsInner,
+ GITS_BASER_OUTER_CACHEABILITY_MASK);
+ reg |= MASK_INSR(cacheability, GITS_BASER_INNER_CACHEABILITY_MASK);
order = get_order_from_bytes(max(ITS_CMD_QUEUE_SZ, SZ_64K));
- buffer = alloc_xenheap_pages(order, gicv3_its_get_memflags());
+ buffer = alloc_xenheap_pages(order, memflags);
if ( !buffer )
return NULL;
@@ -438,8 +424,8 @@ static void *its_map_cbaser(struct host_its *its)
/* The ITS BASE registers work with page sizes of 4K, 16K or 64K. */
#define BASER_PAGE_BITS(sz) ((sz) * 2 + 12)
-static int its_map_baser(void __iomem *basereg, uint64_t regc,
- unsigned int nr_items)
+static int its_map_baser(struct host_its *its, void __iomem *basereg,
+ uint64_t regc, unsigned int nr_items)
{
uint64_t attr, reg;
unsigned int entry_size = GITS_BASER_ENTRY_SIZE(regc);
@@ -447,10 +433,14 @@ static int its_map_baser(void __iomem *basereg, uint64_t regc,
unsigned int table_size;
unsigned int order;
void *buffer;
+ uint64_t cacheability = gicv3_mem_get_cacheability(its->quirk_flags);
+ uint64_t shareability = gicv3_mem_get_shareability(its->quirk_flags);
+ unsigned int memflags = gicv3_mem_get_alloc_flags(its->quirk_flags);
- attr = gicv3_its_get_shareability() << GITS_BASER_SHAREABILITY_SHIFT;
- attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
- attr |= gicv3_its_get_cacheability() << GITS_BASER_INNER_CACHEABILITY_SHIFT;
+ attr = MASK_INSR(shareability, GITS_BASER_SHAREABILITY_MASK);
+ attr |= MASK_INSR(GIC_BASER_CACHE_SameAsInner,
+ GITS_BASER_OUTER_CACHEABILITY_MASK);
+ attr |= MASK_INSR(cacheability, GITS_BASER_INNER_CACHEABILITY_MASK);
/*
* Setup the BASE register with the attributes that we like. Then read
@@ -464,7 +454,7 @@ retry:
table_size = min(table_size, 256U << BASER_PAGE_BITS(pagesz));
order = get_order_from_bytes(max(table_size, BIT(BASER_PAGE_BITS(pagesz), U)));
- buffer = alloc_xenheap_pages(order, gicv3_its_get_memflags());
+ buffer = alloc_xenheap_pages(order, memflags);
if ( !buffer )
return -ENOMEM;
@@ -562,7 +552,7 @@ static int __init gicv3_its_prepare_single_its(struct host_its *hw_its)
if ( ret )
return ret;
- gicv3_its_enable_quirks(hw_its);
+ gicv3_its_collect_quirks(hw_its);
return 0;
}
@@ -592,18 +582,19 @@ static int __init gicv3_its_init_single_its(struct host_its *hw_its)
case GITS_BASER_TYPE_NONE:
continue;
case GITS_BASER_TYPE_DEVICE:
- ret = its_map_baser(basereg, reg, BIT(hw_its->devid_bits, UL));
+ ret = its_map_baser(hw_its, basereg, reg,
+ BIT(hw_its->devid_bits, UL));
if ( ret )
return ret;
break;
case GITS_BASER_TYPE_COLLECTION:
- ret = its_map_baser(basereg, reg, num_possible_cpus());
+ ret = its_map_baser(hw_its, basereg, reg, num_possible_cpus());
if ( ret )
return ret;
break;
/* In case this is a GICv4, provide a (dummy) vPE table as well. */
case GITS_BASER_TYPE_VCPU:
- ret = its_map_baser(basereg, reg, 1);
+ ret = its_map_baser(hw_its, basereg, reg, 1);
if ( ret )
return ret;
break;
@@ -738,6 +729,7 @@ int gicv3_its_map_guest_device(struct domain *d,
struct its_device *dev = NULL;
struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent = NULL;
int i, ret = -ENOENT; /* "i" must be signed to check for >= 0 below. */
+ unsigned int memflags;
unsigned int order;
hw_its = gicv3_its_find_by_doorbell(host_doorbell);
@@ -801,8 +793,9 @@ int gicv3_its_map_guest_device(struct domain *d,
ret = -ENOMEM;
/* An Interrupt Translation Table needs to be 256-byte aligned. */
+ memflags = gicv3_mem_get_alloc_flags(hw_its->quirk_flags);
order = get_order_from_bytes(max(nr_events * hw_its->itte_size, 256UL));
- itt_addr = alloc_xenheap_pages(order, gicv3_its_get_memflags());
+ itt_addr = alloc_xenheap_pages(order, memflags);
if ( !itt_addr )
goto out_unlock;
@@ -1214,8 +1207,6 @@ int __init gicv3_its_init(unsigned int host_lpi_bits)
return ret;
}
- gicv3_its_validate_quirks();
-
if ( list_empty(&host_its_list) )
return 0;
diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
index 9ee338edc2..accfd48a74 100644
--- a/xen/arch/arm/gic-v3-lpi.c
+++ b/xen/arch/arm/gic-v3-lpi.c
@@ -8,6 +8,7 @@
*/
#include <xen/cpu.h>
+#include <xen/init.h>
#include <xen/lib.h>
#include <xen/mm.h>
#include <xen/param.h>
@@ -78,9 +79,29 @@ struct lpi_redist_data {
static DEFINE_PER_CPU(struct lpi_redist_data, lpi_redist);
+/*
+ * Host LPI flags are scoped to shared LPI/Redistributor state, not to an
+ * ITS instance. Arm IHI 0069H.b section 5.1.1 says "LPI configuration is
+ * global". Section 12.11.33 (GICR_PROPBASER) makes mismatched values
+ * UNPREDICTABLE for Redistributors that share an LPI Configuration table
+ * while GICR_CTLR.EnableLPIs == 1. Section 12.11.32 (GICR_PENDBASER)
+ * similarly makes mismatched OuterCache, Shareability or InnerCache values
+ * across Redistributors UNPREDICTABLE while GICR_CTLR.EnableLPIs == 1.
+ *
+ * Keep this policy in the LPI code and accumulate only explicit LPI/RD
+ * restrictions into it. Per-ITS restrictions stay in host_its::quirk_flags
+ * for GITS_CBASER, GITS_BASER<n> and ITT memory.
+ */
+static uint32_t __ro_after_init host_lpi_flags;
+
#define MAX_NR_HOST_LPIS (lpi_data.max_host_lpi_ids - LPI_OFFSET)
#define HOST_LPIS_PER_PAGE (PAGE_SIZE / sizeof(union host_lpi))
+void __init gicv3_lpi_update_host_flags(uint32_t flags)
+{
+ host_lpi_flags |= flags;
+}
+
static union host_lpi *gic_get_host_lpi(uint32_t plpi)
{
union host_lpi *block;
@@ -228,6 +249,7 @@ static int gicv3_lpi_allocate_pendtable(unsigned int cpu)
{
void *pendtable;
unsigned int order;
+ unsigned int memflags = gicv3_mem_get_alloc_flags(host_lpi_flags);
if ( per_cpu(lpi_redist, cpu).pending_table )
return -EBUSY;
@@ -239,7 +261,7 @@ static int gicv3_lpi_allocate_pendtable(unsigned int cpu)
* physically contiguous memory.
*/
order = get_order_from_bytes(max(lpi_data.max_host_lpi_ids / 8, (unsigned long)SZ_64K));
- pendtable = alloc_xenheap_pages(order, gicv3_its_get_memflags());
+ pendtable = alloc_xenheap_pages(order, memflags);
if ( !pendtable )
return -ENOMEM;
@@ -262,6 +284,8 @@ static int gicv3_lpi_set_pendtable(void __iomem *rdist_base)
{
const void *pendtable = this_cpu(lpi_redist).pending_table;
uint64_t val;
+ uint64_t cacheability = gicv3_mem_get_cacheability(host_lpi_flags);
+ uint64_t shareability = gicv3_mem_get_shareability(host_lpi_flags);
/*
* The memory should have been allocated while preparing the CPU (or
@@ -275,9 +299,10 @@ static int gicv3_lpi_set_pendtable(void __iomem *rdist_base)
ASSERT(!(virt_to_maddr(pendtable) & ~GENMASK(51, 16)));
- val = gicv3_its_get_cacheability() << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT;
- val |= GIC_BASER_CACHE_SameAsInner << GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT;
- val |= gicv3_its_get_shareability() << GICR_PENDBASER_SHAREABILITY_SHIFT;
+ val = MASK_INSR(cacheability, GICR_PENDBASER_INNER_CACHEABILITY_MASK);
+ val |= MASK_INSR(GIC_BASER_CACHE_SameAsInner,
+ GICR_PENDBASER_OUTER_CACHEABILITY_MASK);
+ val |= MASK_INSR(shareability, GICR_PENDBASER_SHAREABILITY_MASK);
val |= GICR_PENDBASER_PTZ;
val |= virt_to_maddr(pendtable);
@@ -304,10 +329,13 @@ static int gicv3_lpi_set_proptable(void __iomem * rdist_base)
{
uint64_t reg;
unsigned int order;
+ uint64_t cacheability = gicv3_mem_get_cacheability(host_lpi_flags);
+ uint64_t shareability = gicv3_mem_get_shareability(host_lpi_flags);
- reg = gicv3_its_get_cacheability() << GICR_PROPBASER_INNER_CACHEABILITY_SHIFT;
- reg |= GIC_BASER_CACHE_SameAsInner << GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT;
- reg |= gicv3_its_get_shareability() << GICR_PROPBASER_SHAREABILITY_SHIFT;
+ reg = MASK_INSR(cacheability, GICR_PROPBASER_INNER_CACHEABILITY_MASK);
+ reg |= MASK_INSR(GIC_BASER_CACHE_SameAsInner,
+ GICR_PROPBASER_OUTER_CACHEABILITY_MASK);
+ reg |= MASK_INSR(shareability, GICR_PROPBASER_SHAREABILITY_MASK);
/*
* The property table is shared across all redistributors, so allocate
@@ -317,9 +345,10 @@ static int gicv3_lpi_set_proptable(void __iomem * rdist_base)
{
/* The property table holds one byte per LPI. */
void *table;
+ unsigned int memflags = gicv3_mem_get_alloc_flags(host_lpi_flags);
order = get_order_from_bytes(max(lpi_data.max_host_lpi_ids, (unsigned long)SZ_4K));
- table = alloc_xenheap_pages(order, gicv3_its_get_memflags());
+ table = alloc_xenheap_pages(order, memflags);
if ( !table )
return -ENOMEM;
diff --git a/xen/arch/arm/include/asm/gic_v3_its.h b/xen/arch/arm/include/asm/gic_v3_its.h
index e1322b94f8..9230705181 100644
--- a/xen/arch/arm/include/asm/gic_v3_its.h
+++ b/xen/arch/arm/include/asm/gic_v3_its.h
@@ -110,8 +110,9 @@
#define HOST_ITS_FLUSH_CMD_QUEUE (1U << 0)
#define HOST_ITS_USES_PTA (1U << 1)
-#define HOST_ITS_WORKAROUND_NC_NS (1U << 0)
-#define HOST_ITS_WORKAROUND_32BIT_ADDR (1U << 1)
+/* GICv3 memory-related quirk flags. */
+#define GICV3_QUIRK_MEM_NC_NS (1U << 0)
+#define GICV3_QUIRK_MEM_32BIT_ADDR (1U << 1)
/* We allocate LPIs on the hosts in chunks of 32 to reduce handling overhead. */
#define LPI_BLOCK 32U
@@ -128,6 +129,11 @@ struct host_its {
unsigned int itte_size;
spinlock_t cmd_lock;
void *cmd_buf;
+ /*
+ * Workaround flags scoped to this ITS instance, including memory
+ * accessed through GITS_CBASER, GITS_BASER<n> and ITT memory.
+ */
+ uint32_t quirk_flags;
unsigned int flags;
};
@@ -157,6 +163,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_its_init(unsigned int host_lpi_bits);
+void gicv3_lpi_update_host_flags(uint32_t flags);
/* Store the physical address and ID for each redistributor as read from DT. */
void gicv3_set_redist_address(paddr_t address, unsigned int redist_id);
@@ -199,10 +206,9 @@ struct pending_irq *gicv3_assign_guest_event(struct domain *d,
void gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id,
uint32_t virt_lpi);
-/* ITS quirks handling. */
-uint64_t gicv3_its_get_cacheability(void);
-uint64_t gicv3_its_get_shareability(void);
-unsigned int gicv3_its_get_memflags(void);
+uint64_t gicv3_mem_get_cacheability(uint32_t flags);
+uint64_t gicv3_mem_get_shareability(uint32_t flags);
+unsigned int gicv3_mem_get_alloc_flags(uint32_t flags);
#else
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 3/4] xen/arm: its: refactor ITS quirk matching
2026-09-22 15:58 [PATCH v3 0/4] xen/arm: gicv3: defer host LPI init and split ITS/LPI quirk scopes Mykola Kvach
2026-09-22 15:58 ` [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes Mykola Kvach
2026-09-22 15:58 ` [PATCH v3 2/4] xen/arm: its: separate ITS and host LPI quirk scopes Mykola Kvach
@ 2026-09-22 15:58 ` Mykola Kvach
2026-09-23 0:21 ` Volodymyr Babchuk
2026-09-22 15:58 ` [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes Mykola Kvach
3 siblings, 1 reply; 16+ messages in thread
From: Mykola Kvach @ 2026-09-22 15:58 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk
ITS quirks are matched only by IIDR and mask fields stored in each table
entry. That is too coarse when the same GIC IP block appears on several
platforms but a workaround is valid only for some of them.
Replace the fixed IIDR fields with a generic match(hw_its, data)
callback and an opaque data pointer. Add an IIDR matcher as a reusable
building block and use it from the R-Car Gen4 matcher after checking the
Renesas machine compatibles.
This intentionally narrows the R-Car Gen4 quirk. Previously every ITS
with IIDR 0x0201743b matched. Now it matches only a DT-discovered ITS on
an r8a779f0 or r8a779g0 machine. ACPI-discovered ITSes and the same IIDR
on other platforms no longer match.
Keep first-match semantics explicit. Assert that non-sentinel entries
provide a matcher and that IIDR matching receives match data. Retain
runtime guards so malformed entries cannot cause a NULL function call
or data dereference in non-debug builds. Place the matcher data and
table in init-only read-only sections.
The matched entry still supplies separate ITS and LPI flags; this patch
only changes how the entry is selected.
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
Changes in v3:
- Document the intentional narrowing of the R-Car Gen4 match.
- Add the non-debug NULL-data guard.
- Put the matcher data and table in init-only read-only sections.
Changes in v2:
- Replace v1's optional platform callback plus fixed IIDR/mask fields
with a single generic match(hw_its, data) selector.
- Add a reusable IIDR matcher and use it after the R-Car Gen4
machine-compatible checks.
- Document that the R-Car Gen4 quirk remains DT-only.
- Keep the split ITS and host LPI quirk scopes when applying the matched
entry.
- Document first-match ordering in the lookup path and guard against
entries without a match callback or IIDR match data.
---
xen/arch/arm/gic-v3-its.c | 73 +++++++++++++++++++++++++++++++--------
1 file changed, 58 insertions(+), 15 deletions(-)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 454fd0e0ef..f52232ca40 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -54,8 +54,8 @@ struct its_device {
struct its_quirk {
const char *desc;
- uint32_t iidr;
- uint32_t mask;
+ bool (*match)(const struct host_its *hw_its, const void *data);
+ const void *data;
uint32_t its_flags;
/*
* lpi_flags are ORed into the global host LPI policy and must only
@@ -65,11 +65,52 @@ struct its_quirk {
uint32_t lpi_flags;
};
-static const struct its_quirk its_quirks[] = {
+struct its_quirk_match_iidr {
+ uint32_t iidr;
+ uint32_t mask;
+};
+
+static bool __init gicv3_its_match_iidr(const struct host_its *hw_its,
+ const void *data)
+{
+ const struct its_quirk_match_iidr *match;
+ uint32_t iidr;
+
+ ASSERT(data);
+
+ if ( !data )
+ return false;
+
+ match = data;
+ iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
+
+ return (iidr & match->mask) == match->iidr;
+}
+
+static bool __init gicv3_its_match_quirk_gen4(const struct host_its *hw_its,
+ const void *data)
+{
+ if ( !hw_its->dt_node )
+ return false;
+
+ if ( !dt_machine_is_compatible("renesas,r8a779f0") &&
+ !dt_machine_is_compatible("renesas,r8a779g0") )
+ return false;
+
+ return gicv3_its_match_iidr(hw_its, data);
+}
+
+static const struct its_quirk_match_iidr rcar_gen4_iidr __initconst = {
+ /* Implementer 0x43b identifies Arm Ltd. */
+ .iidr = 0x0201743b,
+ .mask = 0xffffffffU,
+};
+
+static const struct its_quirk its_quirks[] __initconstrel = {
{
- .desc = "R-Car Gen4",
- .iidr = 0x0201743b,
- .mask = 0xffffffffU,
+ .desc = "R-Car Gen4",
+ .match = gicv3_its_match_quirk_gen4,
+ .data = &rcar_gen4_iidr,
.its_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
.lpi_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
},
@@ -78,18 +119,21 @@ static const struct its_quirk its_quirks[] = {
}
};
-static const struct its_quirk *__init gicv3_its_find_quirk(uint32_t iidr)
+static const struct its_quirk *__init gicv3_its_find_quirk(
+ const struct host_its *hw_its)
{
- const struct its_quirk *quirks = its_quirks;
+ const struct its_quirk *quirk;
/*
- * The first matching quirk wins. More specific quirks must be listed
- * before broader IIDR-only entries.
+ * The first matching quirk wins. Entries that match a specific platform
+ * must be listed before broader IIDR-only entries.
*/
- for ( ; quirks->desc; quirks++ )
+ for ( quirk = its_quirks; quirk->desc; quirk++ )
{
- if ( quirks->iidr == (quirks->mask & iidr) )
- return quirks;
+ ASSERT(quirk->match);
+
+ if ( quirk->match && quirk->match(hw_its, quirk->data) )
+ return quirk;
}
return NULL;
@@ -97,8 +141,7 @@ static const struct its_quirk *__init gicv3_its_find_quirk(uint32_t iidr)
static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
{
- uint32_t iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
- const struct its_quirk *quirk = gicv3_its_find_quirk(iidr);
+ const struct its_quirk *quirk = gicv3_its_find_quirk(hw_its);
if ( quirk )
{
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes
2026-09-22 15:58 [PATCH v3 0/4] xen/arm: gicv3: defer host LPI init and split ITS/LPI quirk scopes Mykola Kvach
` (2 preceding siblings ...)
2026-09-22 15:58 ` [PATCH v3 3/4] xen/arm: its: refactor ITS quirk matching Mykola Kvach
@ 2026-09-22 15:58 ` Mykola Kvach
2026-09-23 0:29 ` Volodymyr Babchuk
3 siblings, 1 reply; 16+ messages in thread
From: Mykola Kvach @ 2026-09-22 15:58 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk
The DT dma-noncoherent property describes the bus coherency of the
device represented by the node. On an ITS subnode, that is memory
accessed by that ITS. Add GICV3_QUIRK_MEM_NC_NS to the corresponding
host_its and use it when programming GITS_CBASER and GITS_BASER<n>.
On the top-level GIC node, the property describes the Redistributor side
of the LPI path. Collect it in gicv3_lpi_init_host_lpis() and apply it
only to the host LPI policy used when allocating the property and
pending tables and when programming GICR_PROPBASER and GICR_PENDBASER.
Mark the function init-only because firmware attributes are collected
only during boot.
Do not inherit the property between parent and child nodes: ITS-node
non-coherency does not change the global host LPI policy, and GIC-node
non-coherency does not change per-ITS quirk_flags.
ACPI is unchanged; this patch only consumes the DT dma-noncoherent
property.
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
Changes in v3:
- Keep the ITS init annotations with the earlier initialization patch.
- Drop the redundant aggregate host-LPI-flags message.
Changes in v2:
- Split v1's dma-noncoherent handling into explicit ITS-node and GIC-node
scopes.
- Apply an ITS subnode property only to the matching host_its
quirk_flags.
- Collect the top-level GIC property from gic-v3-lpi.c before host LPI
allocations use host_lpi_flags.
---
xen/arch/arm/gic-v3-its.c | 17 +++++++++++++++++
xen/arch/arm/gic-v3-lpi.c | 20 +++++++++++++++++++-
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index f52232ca40..6734f94e7c 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -139,6 +139,21 @@ static const struct its_quirk *__init gicv3_its_find_quirk(
return NULL;
}
+static void __init gicv3_its_collect_fw_attrs(struct host_its *hw_its)
+{
+ /*
+ * An ITS subnode property describes memory transactions made by that ITS.
+ * Do not inherit it into the global host LPI/Redistributor policy.
+ */
+ if ( !hw_its->dt_node ||
+ !dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
+ return;
+
+ hw_its->quirk_flags |= GICV3_QUIRK_MEM_NC_NS;
+ printk("GICv3: ITS @%#"PRIpaddr" marked dma-noncoherent\n",
+ hw_its->addr);
+}
+
static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
{
const struct its_quirk *quirk = gicv3_its_find_quirk(hw_its);
@@ -149,6 +164,8 @@ static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
gicv3_lpi_update_host_flags(quirk->lpi_flags);
printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
}
+
+ gicv3_its_collect_fw_attrs(hw_its);
}
uint64_t gicv3_mem_get_cacheability(uint32_t flags)
diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
index accfd48a74..1cf49430ea 100644
--- a/xen/arch/arm/gic-v3-lpi.c
+++ b/xen/arch/arm/gic-v3-lpi.c
@@ -7,7 +7,9 @@
* Copyright (C) 2016,2017 - ARM Ltd
*/
+#include <xen/acpi.h>
#include <xen/cpu.h>
+#include <xen/device_tree.h>
#include <xen/init.h>
#include <xen/lib.h>
#include <xen/mm.h>
@@ -102,6 +104,20 @@ void __init gicv3_lpi_update_host_flags(uint32_t flags)
host_lpi_flags |= flags;
}
+static void __init gicv3_lpi_collect_fw_attrs(void)
+{
+ /*
+ * A top-level GIC node property describes the Redistributor side of the
+ * LPI path. Do not inherit it into per-ITS policy.
+ */
+ if ( !acpi_disabled ||
+ !dt_property_read_bool(dt_interrupt_controller, "dma-noncoherent") )
+ return;
+
+ gicv3_lpi_update_host_flags(GICV3_QUIRK_MEM_NC_NS);
+ printk("GICv3: Redistributors marked dma-noncoherent\n");
+}
+
static union host_lpi *gic_get_host_lpi(uint32_t plpi)
{
union host_lpi *block;
@@ -443,7 +459,7 @@ integer_param("max_lpi_bits", max_lpi_bits);
* to the page with the actual "union host_lpi" entries. Our LPI limit
* avoids excessive memory usage.
*/
-int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
+int __init gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
{
unsigned int nr_lpi_ptrs;
int rc;
@@ -451,6 +467,8 @@ int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
/* We rely on the data structure being atomically accessible. */
BUILD_BUG_ON(sizeof(union host_lpi) > sizeof(unsigned long));
+ gicv3_lpi_collect_fw_attrs();
+
/*
* An implementation needs to support at least 14 bits of LPI IDs.
* Tell the user about it, the actual number is reported below.
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes
2026-09-22 15:58 ` [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes Mykola Kvach
@ 2026-09-22 23:56 ` Volodymyr Babchuk
2026-09-24 5:00 ` Mykola Kvach
0 siblings, 1 reply; 16+ messages in thread
From: Volodymyr Babchuk @ 2026-09-22 23:56 UTC (permalink / raw)
To: Mykola Kvach
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel
Hi Mykola,
Mykola Kvach <mykola_kvach@epam.com> writes:
> The boot CPU pending table must use the memory attributes selected by
> ITS quirks. gicv3_lpi_init_host_lpis() is therefore called after
> gicv3_its_init(). However, gicv3_its_init() also programs and enables
> each ITS before host LPI state is allocated. No ITS commands are
> submitted at that point, but this ordering relies on that implementation
> detail.
>
> Split per-ITS initialization into preparation and activation phases.
> First map and disable every ITS and collect its quirks. Then initialize
> host LPI state. Only after that, allocate and program the ITS tables and
> command queue, and enable each ITS.
>
> The subsequent gicv3_cpu_init() sequence remains unchanged: it programs
> the Redistributor LPI tables, enables LPIs, and then submits the first
> MAPC and SYNC commands.
>
> Suggested-by: Julien Grall <julien@xen.org>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> Changes in v3:
> - New patch implementing the post-4.22 initialization order discussed
> during review of the ordering fix.
>
> Link: https://patchew.org/Xen/341edd8de63dcd84ccc6e7b6c03e9e8fc7105184.1781847061.git.mykola._5Fkvach@epam.com/
> ---
> xen/arch/arm/gic-v3-its.c | 32 ++++++++++++++++++++++-----
> xen/arch/arm/gic-v3.c | 15 ++-----------
> xen/arch/arm/include/asm/gic_v3_its.h | 4 ++--
> 3 files changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 325835b0ad..972825bf06 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -11,6 +11,7 @@
> #include <xen/lib.h>
> #include <xen/delay.h>
> #include <xen/iocap.h>
> +#include <xen/init.h>
> #include <xen/libfdt/libfdt.h>
> #include <xen/mm.h>
> #include <xen/rbtree.h>
> @@ -549,10 +550,9 @@ static int gicv3_disable_its(struct host_its *hw_its)
> return -ETIMEDOUT;
> }
>
> -static int gicv3_its_init_single_its(struct host_its *hw_its)
> +static int __init gicv3_its_prepare_single_its(struct host_its *hw_its)
> {
> - uint64_t reg;
> - int i, ret;
> + int ret;
>
> hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
> if ( !hw_its->its_base )
> @@ -564,6 +564,14 @@ static int gicv3_its_init_single_its(struct host_its *hw_its)
>
> gicv3_its_enable_quirks(hw_its);
>
> + return 0;
> +}
> +
> +static int __init gicv3_its_init_single_its(struct host_its *hw_its)
> +{
> + uint64_t reg;
> + int i, ret;
> +
> 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);
> @@ -1189,7 +1197,7 @@ static void gicv3_its_acpi_init(void)
>
> #endif
>
> -int gicv3_its_init(void)
> +int __init gicv3_its_init(unsigned int host_lpi_bits)
> {
> struct host_its *hw_its;
> int ret;
> @@ -1201,13 +1209,27 @@ int gicv3_its_init(void)
>
> list_for_each_entry(hw_its, &host_its_list, entry)
> {
> - ret = gicv3_its_init_single_its(hw_its);
> + ret = gicv3_its_prepare_single_its(hw_its);
> if ( ret )
> return ret;
> }
>
> gicv3_its_validate_quirks();
>
> + if ( list_empty(&host_its_list) )
> + return 0;
What is the purpose of this check here? I'd expect to see it before the
first list_for_each_entry() loop.
> +
> + ret = gicv3_lpi_init_host_lpis(host_lpi_bits);
> + if ( ret )
> + return ret;
> +
> + list_for_each_entry(hw_its, &host_its_list, entry)
> + {
> + ret = gicv3_its_init_single_its(hw_its);
> + if ( ret )
> + return ret;
> + }
> +
> return 0;
> }
[...]
--
WBR, Volodymyr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/4] xen/arm: its: separate ITS and host LPI quirk scopes
2026-09-22 15:58 ` [PATCH v3 2/4] xen/arm: its: separate ITS and host LPI quirk scopes Mykola Kvach
@ 2026-09-23 0:13 ` Volodymyr Babchuk
0 siblings, 0 replies; 16+ messages in thread
From: Volodymyr Babchuk @ 2026-09-23 0:13 UTC (permalink / raw)
To: Mykola Kvach
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel
Hi Mykola,
Mykola Kvach <mykola_kvach@epam.com> writes:
> ITS quirks can impose restrictions on memory accessed by the ITS itself
> and on shared host LPI/Redistributor state. These scopes are not
> identical, so a single global ITS quirk state makes the host LPI policy
> depend implicitly on the quirks seen while initializing host ITSes.
>
> Add per-ITS quirk_flags to struct host_its and keep a separate
> host_lpi_flags state in the LPI code. The quirk table now records the
> ITS-private and host LPI scopes explicitly through its_flags and
> lpi_flags. The R-Car Gen4 quirk applies the same memory restrictions to
> both scopes, preserving the existing R-Car memory policy without relying
> on an implicit aggregation step.
>
> This also removes the assumption that all host ITSes must expose the
> same quirk state. ITS-derived host LPI restrictions are accumulated only
> from quirk entries that explicitly set lpi_flags.
>
> Use per-ITS quirk_flags for GITS_CBASER, GITS_BASER<n>, and ITT
> allocations. Use host_lpi_flags when allocating the property and pending
> tables and when programming GICR_PROPBASER and GICR_PENDBASER.
> Memory-related quirk bits are named GICV3_QUIRK_MEM_* and are translated
> by shared gicv3_mem_get_*() helpers.
>
> Collect both scopes during the ITS preparation pass, before host LPI
> state is allocated. Keep init-only annotations and includes in the
> implementation files rather than the public header.
>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> Changes in v3:
> - Collect quirk flags during the ITS preparation pass added earlier.
> - Keep init-only annotations and includes in implementation files.
>
> Changes in v2:
> - Replace v1's single global ITS quirk flags and same-quirk validation
> with explicit per-ITS and host LPI quirk scopes.
> - Drop the v1 ITS pre-initialization approach from this patch. In v2,
> host LPI allocation was moved after ITS initialization by a separate
> patch.
> - Drop DT dma-noncoherent handling from this patch; firmware-provided
> non-coherency is handled separately with explicit ITS-node and
> GIC-node scopes.
> - Keep host_lpi_flags owned by gic-v3-lpi.c and update it only with quirk
> flags that explicitly apply to host LPI/Redistributor state.
> - Rename the current memory-related bits to GICV3_QUIRK_MEM_* and use
> shared gicv3_mem_get_*() helpers for register attributes and allocation
> flags.
> ---
> xen/arch/arm/gic-v3-its.c | 111 ++++++++++++--------------
> xen/arch/arm/gic-v3-lpi.c | 45 +++++++++--
> xen/arch/arm/include/asm/gic_v3_its.h | 18 +++--
> 3 files changed, 100 insertions(+), 74 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 972825bf06..454fd0e0ef 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -52,43 +52,40 @@ struct its_device {
> struct pending_irq *pend_irqs; /* One struct per event */
> };
>
> -/*
> - * It is unlikely that a platform implements ITSes with different quirks,
> - * so assume they all share the same.
> - */
> struct its_quirk {
> const char *desc;
> - bool (*init)(struct host_its *hw_its);
> uint32_t iidr;
> uint32_t mask;
> + uint32_t its_flags;
> + /*
> + * lpi_flags are ORed into the global host LPI policy and must only
> + * contain additive restrictions. Non-additive LPI quirks need explicit
> + * handling.
> + */
> + uint32_t lpi_flags;
> };
>
> -static uint32_t __ro_after_init its_quirk_flags;
> -
> -static bool gicv3_its_enable_quirk_gen4(struct host_its *hw_its)
> -{
> - its_quirk_flags |= HOST_ITS_WORKAROUND_NC_NS |
> - HOST_ITS_WORKAROUND_32BIT_ADDR;
> -
> - return true;
> -}
> -
> static const struct its_quirk its_quirks[] = {
> {
> .desc = "R-Car Gen4",
> .iidr = 0x0201743b,
> .mask = 0xffffffffU,
> - .init = gicv3_its_enable_quirk_gen4,
> + .its_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
> + .lpi_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
> },
> {
> /* Sentinel. */
> }
> };
>
> -static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
> +static const struct its_quirk *__init gicv3_its_find_quirk(uint32_t iidr)
> {
> const struct its_quirk *quirks = its_quirks;
>
> + /*
> + * The first matching quirk wins. More specific quirks must be listed
> + * before broader IIDR-only entries.
> + */
> for ( ; quirks->desc; quirks++ )
> {
> if ( quirks->iidr == (quirks->mask & iidr) )
> @@ -98,53 +95,38 @@ static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
> return NULL;
> }
>
> -static void gicv3_its_enable_quirks(struct host_its *hw_its)
> +static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
> {
> uint32_t iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
> const struct its_quirk *quirk = gicv3_its_find_quirk(iidr);
>
> - if ( quirk && quirk->init(hw_its) )
> - printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
> -}
> -
> -static void gicv3_its_validate_quirks(void)
> -{
> - const struct its_quirk *quirk = NULL, *prev = NULL;
> - 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);
> - prev = gicv3_its_find_quirk(readl_relaxed(hw_its->its_base + GITS_IIDR));
> -
> - list_for_each_entry(hw_its, &host_its_list, entry)
> + if ( quirk )
> {
> - quirk = gicv3_its_find_quirk(readl_relaxed(hw_its->its_base + GITS_IIDR));
> - BUG_ON(quirk != prev);
> - prev = quirk;
> + hw_its->quirk_flags |= quirk->its_flags;
> + gicv3_lpi_update_host_flags(quirk->lpi_flags);
> + printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
> }
> }
>
> -uint64_t gicv3_its_get_cacheability(void)
> +uint64_t gicv3_mem_get_cacheability(uint32_t flags)
> {
> - if ( its_quirk_flags & HOST_ITS_WORKAROUND_NC_NS )
> + if ( flags & GICV3_QUIRK_MEM_NC_NS )
> return GIC_BASER_CACHE_nC;
>
> return GIC_BASER_CACHE_RaWaWb;
> }
>
> -uint64_t gicv3_its_get_shareability(void)
> +uint64_t gicv3_mem_get_shareability(uint32_t flags)
> {
> - if ( its_quirk_flags & HOST_ITS_WORKAROUND_NC_NS )
> + if ( flags & GICV3_QUIRK_MEM_NC_NS )
> return GIC_BASER_NonShareable;
>
> return GIC_BASER_InnerShareable;
> }
>
> -unsigned int gicv3_its_get_memflags(void)
> +unsigned int gicv3_mem_get_alloc_flags(uint32_t flags)
> {
> - if ( its_quirk_flags & HOST_ITS_WORKAROUND_32BIT_ADDR )
> + if ( flags & GICV3_QUIRK_MEM_32BIT_ADDR )
> return MEMF_bits(32);
>
> return 0;
> @@ -391,13 +373,17 @@ static void *its_map_cbaser(struct host_its *its)
> uint64_t reg;
> unsigned int order;
> void *buffer;
> + uint64_t cacheability = gicv3_mem_get_cacheability(its->quirk_flags);
> + uint64_t shareability = gicv3_mem_get_shareability(its->quirk_flags);
> + unsigned int memflags = gicv3_mem_get_alloc_flags(its->quirk_flags);
>
> - reg = gicv3_its_get_shareability() << GITS_BASER_SHAREABILITY_SHIFT;
> - reg |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
> - reg |= gicv3_its_get_cacheability() << GITS_BASER_INNER_CACHEABILITY_SHIFT;
> + reg = MASK_INSR(shareability, GITS_BASER_SHAREABILITY_MASK);
> + reg |= MASK_INSR(GIC_BASER_CACHE_SameAsInner,
> + GITS_BASER_OUTER_CACHEABILITY_MASK);
> + reg |= MASK_INSR(cacheability, GITS_BASER_INNER_CACHEABILITY_MASK);
>
> order = get_order_from_bytes(max(ITS_CMD_QUEUE_SZ, SZ_64K));
> - buffer = alloc_xenheap_pages(order, gicv3_its_get_memflags());
> + buffer = alloc_xenheap_pages(order, memflags);
> if ( !buffer )
> return NULL;
>
> @@ -438,8 +424,8 @@ static void *its_map_cbaser(struct host_its *its)
> /* The ITS BASE registers work with page sizes of 4K, 16K or 64K. */
> #define BASER_PAGE_BITS(sz) ((sz) * 2 + 12)
>
> -static int its_map_baser(void __iomem *basereg, uint64_t regc,
> - unsigned int nr_items)
> +static int its_map_baser(struct host_its *its, void __iomem *basereg,
> + uint64_t regc, unsigned int nr_items)
> {
> uint64_t attr, reg;
> unsigned int entry_size = GITS_BASER_ENTRY_SIZE(regc);
> @@ -447,10 +433,14 @@ static int its_map_baser(void __iomem *basereg, uint64_t regc,
> unsigned int table_size;
> unsigned int order;
> void *buffer;
> + uint64_t cacheability = gicv3_mem_get_cacheability(its->quirk_flags);
> + uint64_t shareability = gicv3_mem_get_shareability(its->quirk_flags);
> + unsigned int memflags = gicv3_mem_get_alloc_flags(its->quirk_flags);
>
> - attr = gicv3_its_get_shareability() << GITS_BASER_SHAREABILITY_SHIFT;
> - attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
> - attr |= gicv3_its_get_cacheability() << GITS_BASER_INNER_CACHEABILITY_SHIFT;
> + attr = MASK_INSR(shareability, GITS_BASER_SHAREABILITY_MASK);
> + attr |= MASK_INSR(GIC_BASER_CACHE_SameAsInner,
> + GITS_BASER_OUTER_CACHEABILITY_MASK);
> + attr |= MASK_INSR(cacheability, GITS_BASER_INNER_CACHEABILITY_MASK);
>
> /*
> * Setup the BASE register with the attributes that we like. Then read
> @@ -464,7 +454,7 @@ retry:
> table_size = min(table_size, 256U << BASER_PAGE_BITS(pagesz));
>
> order = get_order_from_bytes(max(table_size, BIT(BASER_PAGE_BITS(pagesz), U)));
> - buffer = alloc_xenheap_pages(order, gicv3_its_get_memflags());
> + buffer = alloc_xenheap_pages(order, memflags);
> if ( !buffer )
> return -ENOMEM;
>
> @@ -562,7 +552,7 @@ static int __init gicv3_its_prepare_single_its(struct host_its *hw_its)
> if ( ret )
> return ret;
>
> - gicv3_its_enable_quirks(hw_its);
> + gicv3_its_collect_quirks(hw_its);
>
> return 0;
> }
> @@ -592,18 +582,19 @@ static int __init gicv3_its_init_single_its(struct host_its *hw_its)
> case GITS_BASER_TYPE_NONE:
> continue;
> case GITS_BASER_TYPE_DEVICE:
> - ret = its_map_baser(basereg, reg, BIT(hw_its->devid_bits, UL));
> + ret = its_map_baser(hw_its, basereg, reg,
> + BIT(hw_its->devid_bits, UL));
You are passing hw_its now, so there is no need to pass the last
argument, because its_map_baser() can calculate it by itself.
> if ( ret )
> return ret;
> break;
> case GITS_BASER_TYPE_COLLECTION:
> - ret = its_map_baser(basereg, reg, num_possible_cpus());
> + ret = its_map_baser(hw_its, basereg, reg, num_possible_cpus());
> if ( ret )
> return ret;
> break;
> /* In case this is a GICv4, provide a (dummy) vPE table as well. */
> case GITS_BASER_TYPE_VCPU:
> - ret = its_map_baser(basereg, reg, 1);
> + ret = its_map_baser(hw_its, basereg, reg, 1);
> if ( ret )
> return ret;
> break;
> @@ -738,6 +729,7 @@ int gicv3_its_map_guest_device(struct domain *d,
> struct its_device *dev = NULL;
> struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent = NULL;
> int i, ret = -ENOENT; /* "i" must be signed to check for >= 0 below. */
> + unsigned int memflags;
> unsigned int order;
>
> hw_its = gicv3_its_find_by_doorbell(host_doorbell);
> @@ -801,8 +793,9 @@ int gicv3_its_map_guest_device(struct domain *d,
> ret = -ENOMEM;
>
> /* An Interrupt Translation Table needs to be 256-byte aligned. */
> + memflags = gicv3_mem_get_alloc_flags(hw_its->quirk_flags);
> order = get_order_from_bytes(max(nr_events * hw_its->itte_size, 256UL));
> - itt_addr = alloc_xenheap_pages(order, gicv3_its_get_memflags());
> + itt_addr = alloc_xenheap_pages(order, memflags);
> if ( !itt_addr )
> goto out_unlock;
>
> @@ -1214,8 +1207,6 @@ int __init gicv3_its_init(unsigned int host_lpi_bits)
> return ret;
> }
>
> - gicv3_its_validate_quirks();
> -
> if ( list_empty(&host_its_list) )
> return 0;
>
> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> index 9ee338edc2..accfd48a74 100644
> --- a/xen/arch/arm/gic-v3-lpi.c
> +++ b/xen/arch/arm/gic-v3-lpi.c
> @@ -8,6 +8,7 @@
> */
>
> #include <xen/cpu.h>
> +#include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/mm.h>
> #include <xen/param.h>
> @@ -78,9 +79,29 @@ struct lpi_redist_data {
>
> static DEFINE_PER_CPU(struct lpi_redist_data, lpi_redist);
>
> +/*
> + * Host LPI flags are scoped to shared LPI/Redistributor state, not to an
> + * ITS instance. Arm IHI 0069H.b section 5.1.1 says "LPI configuration is
> + * global". Section 12.11.33 (GICR_PROPBASER) makes mismatched values
> + * UNPREDICTABLE for Redistributors that share an LPI Configuration table
> + * while GICR_CTLR.EnableLPIs == 1. Section 12.11.32 (GICR_PENDBASER)
> + * similarly makes mismatched OuterCache, Shareability or InnerCache values
> + * across Redistributors UNPREDICTABLE while GICR_CTLR.EnableLPIs == 1.
> + *
> + * Keep this policy in the LPI code and accumulate only explicit LPI/RD
> + * restrictions into it. Per-ITS restrictions stay in host_its::quirk_flags
> + * for GITS_CBASER, GITS_BASER<n> and ITT memory.
> + */
> +static uint32_t __ro_after_init host_lpi_flags;
> +
> #define MAX_NR_HOST_LPIS (lpi_data.max_host_lpi_ids - LPI_OFFSET)
> #define HOST_LPIS_PER_PAGE (PAGE_SIZE / sizeof(union host_lpi))
>
> +void __init gicv3_lpi_update_host_flags(uint32_t flags)
> +{
> + host_lpi_flags |= flags;
> +}
> +
> static union host_lpi *gic_get_host_lpi(uint32_t plpi)
> {
> union host_lpi *block;
> @@ -228,6 +249,7 @@ static int gicv3_lpi_allocate_pendtable(unsigned int cpu)
> {
> void *pendtable;
> unsigned int order;
> + unsigned int memflags = gicv3_mem_get_alloc_flags(host_lpi_flags);
>
> if ( per_cpu(lpi_redist, cpu).pending_table )
> return -EBUSY;
> @@ -239,7 +261,7 @@ static int gicv3_lpi_allocate_pendtable(unsigned int cpu)
> * physically contiguous memory.
> */
> order = get_order_from_bytes(max(lpi_data.max_host_lpi_ids / 8, (unsigned long)SZ_64K));
> - pendtable = alloc_xenheap_pages(order, gicv3_its_get_memflags());
> + pendtable = alloc_xenheap_pages(order, memflags);
> if ( !pendtable )
> return -ENOMEM;
>
> @@ -262,6 +284,8 @@ static int gicv3_lpi_set_pendtable(void __iomem *rdist_base)
> {
> const void *pendtable = this_cpu(lpi_redist).pending_table;
> uint64_t val;
> + uint64_t cacheability = gicv3_mem_get_cacheability(host_lpi_flags);
> + uint64_t shareability = gicv3_mem_get_shareability(host_lpi_flags);
>
> /*
> * The memory should have been allocated while preparing the CPU (or
> @@ -275,9 +299,10 @@ static int gicv3_lpi_set_pendtable(void __iomem *rdist_base)
>
> ASSERT(!(virt_to_maddr(pendtable) & ~GENMASK(51, 16)));
>
> - val = gicv3_its_get_cacheability() << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT;
> - val |= GIC_BASER_CACHE_SameAsInner << GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT;
> - val |= gicv3_its_get_shareability() << GICR_PENDBASER_SHAREABILITY_SHIFT;
> + val = MASK_INSR(cacheability, GICR_PENDBASER_INNER_CACHEABILITY_MASK);
> + val |= MASK_INSR(GIC_BASER_CACHE_SameAsInner,
> + GICR_PENDBASER_OUTER_CACHEABILITY_MASK);
> + val |= MASK_INSR(shareability, GICR_PENDBASER_SHAREABILITY_MASK);
> val |= GICR_PENDBASER_PTZ;
> val |= virt_to_maddr(pendtable);
>
> @@ -304,10 +329,13 @@ static int gicv3_lpi_set_proptable(void __iomem * rdist_base)
> {
> uint64_t reg;
> unsigned int order;
> + uint64_t cacheability = gicv3_mem_get_cacheability(host_lpi_flags);
> + uint64_t shareability = gicv3_mem_get_shareability(host_lpi_flags);
>
> - reg = gicv3_its_get_cacheability() << GICR_PROPBASER_INNER_CACHEABILITY_SHIFT;
> - reg |= GIC_BASER_CACHE_SameAsInner << GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT;
> - reg |= gicv3_its_get_shareability() << GICR_PROPBASER_SHAREABILITY_SHIFT;
> + reg = MASK_INSR(cacheability, GICR_PROPBASER_INNER_CACHEABILITY_MASK);
> + reg |= MASK_INSR(GIC_BASER_CACHE_SameAsInner,
> + GICR_PROPBASER_OUTER_CACHEABILITY_MASK);
> + reg |= MASK_INSR(shareability, GICR_PROPBASER_SHAREABILITY_MASK);
>
> /*
> * The property table is shared across all redistributors, so allocate
> @@ -317,9 +345,10 @@ static int gicv3_lpi_set_proptable(void __iomem * rdist_base)
> {
> /* The property table holds one byte per LPI. */
> void *table;
> + unsigned int memflags = gicv3_mem_get_alloc_flags(host_lpi_flags);
>
> order = get_order_from_bytes(max(lpi_data.max_host_lpi_ids, (unsigned long)SZ_4K));
> - table = alloc_xenheap_pages(order, gicv3_its_get_memflags());
> + table = alloc_xenheap_pages(order, memflags);
>
> if ( !table )
> return -ENOMEM;
> diff --git a/xen/arch/arm/include/asm/gic_v3_its.h b/xen/arch/arm/include/asm/gic_v3_its.h
> index e1322b94f8..9230705181 100644
> --- a/xen/arch/arm/include/asm/gic_v3_its.h
> +++ b/xen/arch/arm/include/asm/gic_v3_its.h
> @@ -110,8 +110,9 @@
> #define HOST_ITS_FLUSH_CMD_QUEUE (1U << 0)
> #define HOST_ITS_USES_PTA (1U << 1)
>
> -#define HOST_ITS_WORKAROUND_NC_NS (1U << 0)
> -#define HOST_ITS_WORKAROUND_32BIT_ADDR (1U << 1)
> +/* GICv3 memory-related quirk flags. */
> +#define GICV3_QUIRK_MEM_NC_NS (1U << 0)
> +#define GICV3_QUIRK_MEM_32BIT_ADDR (1U << 1)
>
> /* We allocate LPIs on the hosts in chunks of 32 to reduce handling overhead. */
> #define LPI_BLOCK 32U
> @@ -128,6 +129,11 @@ struct host_its {
> unsigned int itte_size;
> spinlock_t cmd_lock;
> void *cmd_buf;
> + /*
> + * Workaround flags scoped to this ITS instance, including memory
> + * accessed through GITS_CBASER, GITS_BASER<n> and ITT memory.
> + */
> + uint32_t quirk_flags;
> unsigned int flags;
> };
>
> @@ -157,6 +163,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_its_init(unsigned int host_lpi_bits);
> +void gicv3_lpi_update_host_flags(uint32_t flags);
>
> /* Store the physical address and ID for each redistributor as read from DT. */
> void gicv3_set_redist_address(paddr_t address, unsigned int redist_id);
> @@ -199,10 +206,9 @@ struct pending_irq *gicv3_assign_guest_event(struct domain *d,
> void gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id,
> uint32_t virt_lpi);
>
> -/* ITS quirks handling. */
Maybe leave the comment? Just change "ITS" to "ITS/LPI" ?
> -uint64_t gicv3_its_get_cacheability(void);
> -uint64_t gicv3_its_get_shareability(void);
> -unsigned int gicv3_its_get_memflags(void);
> +uint64_t gicv3_mem_get_cacheability(uint32_t flags);
> +uint64_t gicv3_mem_get_shareability(uint32_t flags);
> +unsigned int gicv3_mem_get_alloc_flags(uint32_t flags);
>
> #else
--
WBR, Volodymyr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] xen/arm: its: refactor ITS quirk matching
2026-09-22 15:58 ` [PATCH v3 3/4] xen/arm: its: refactor ITS quirk matching Mykola Kvach
@ 2026-09-23 0:21 ` Volodymyr Babchuk
2026-09-24 5:00 ` Mykola Kvach
0 siblings, 1 reply; 16+ messages in thread
From: Volodymyr Babchuk @ 2026-09-23 0:21 UTC (permalink / raw)
To: Mykola Kvach
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel
Hi,
Mykola Kvach <mykola_kvach@epam.com> writes:
> ITS quirks are matched only by IIDR and mask fields stored in each table
> entry. That is too coarse when the same GIC IP block appears on several
> platforms but a workaround is valid only for some of them.
>
> Replace the fixed IIDR fields with a generic match(hw_its, data)
> callback and an opaque data pointer. Add an IIDR matcher as a reusable
> building block and use it from the R-Car Gen4 matcher after checking the
> Renesas machine compatibles.
>
> This intentionally narrows the R-Car Gen4 quirk. Previously every ITS
> with IIDR 0x0201743b matched. Now it matches only a DT-discovered ITS on
> an r8a779f0 or r8a779g0 machine. ACPI-discovered ITSes and the same IIDR
> on other platforms no longer match.
>
> Keep first-match semantics explicit. Assert that non-sentinel entries
> provide a matcher and that IIDR matching receives match data. Retain
> runtime guards so malformed entries cannot cause a NULL function call
> or data dereference in non-debug builds. Place the matcher data and
> table in init-only read-only sections.
>
> The matched entry still supplies separate ITS and LPI flags; this patch
> only changes how the entry is selected.
>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> Changes in v3:
> - Document the intentional narrowing of the R-Car Gen4 match.
> - Add the non-debug NULL-data guard.
> - Put the matcher data and table in init-only read-only sections.
>
> Changes in v2:
> - Replace v1's optional platform callback plus fixed IIDR/mask fields
> with a single generic match(hw_its, data) selector.
> - Add a reusable IIDR matcher and use it after the R-Car Gen4
> machine-compatible checks.
> - Document that the R-Car Gen4 quirk remains DT-only.
> - Keep the split ITS and host LPI quirk scopes when applying the matched
> entry.
> - Document first-match ordering in the lookup path and guard against
> entries without a match callback or IIDR match data.
> ---
> xen/arch/arm/gic-v3-its.c | 73 +++++++++++++++++++++++++++++++--------
> 1 file changed, 58 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 454fd0e0ef..f52232ca40 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -54,8 +54,8 @@ struct its_device {
>
> struct its_quirk {
> const char *desc;
> - uint32_t iidr;
> - uint32_t mask;
> + bool (*match)(const struct host_its *hw_its, const void *data);
> + const void *data;
> uint32_t its_flags;
> /*
> * lpi_flags are ORed into the global host LPI policy and must only
> @@ -65,11 +65,52 @@ struct its_quirk {
> uint32_t lpi_flags;
> };
>
> -static const struct its_quirk its_quirks[] = {
> +struct its_quirk_match_iidr {
> + uint32_t iidr;
> + uint32_t mask;
> +};
> +
> +static bool __init gicv3_its_match_iidr(const struct host_its *hw_its,
> + const void *data)
> +{
> + const struct its_quirk_match_iidr *match;
> + uint32_t iidr;
> +
> + ASSERT(data);
> +
> + if ( !data )
> + return false;
It looks strange to have both ASSERT and then this check.
Maybe it is worth to remove ASSERT and print a warning in a debug build instead?
> +
> + match = data;
> + iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
> +
> + return (iidr & match->mask) == match->iidr;
> +}
> +
> +static bool __init gicv3_its_match_quirk_gen4(const struct host_its *hw_its,
> + const void *data)
> +{
> + if ( !hw_its->dt_node )
> + return false;
> +
> + if ( !dt_machine_is_compatible("renesas,r8a779f0") &&
> + !dt_machine_is_compatible("renesas,r8a779g0") )
> + return false;
> +
> + return gicv3_its_match_iidr(hw_its, data);
Do you really need to match IIDR in this case? You already know that
platform requires the quirk.
> +}
> +
> +static const struct its_quirk_match_iidr rcar_gen4_iidr __initconst = {
> + /* Implementer 0x43b identifies Arm Ltd. */
> + .iidr = 0x0201743b,
> + .mask = 0xffffffffU,
> +};
> +
> +static const struct its_quirk its_quirks[] __initconstrel = {
> {
> - .desc = "R-Car Gen4",
> - .iidr = 0x0201743b,
> - .mask = 0xffffffffU,
> + .desc = "R-Car Gen4",
> + .match = gicv3_its_match_quirk_gen4,
> + .data = &rcar_gen4_iidr,
> .its_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
> .lpi_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
> },
> @@ -78,18 +119,21 @@ static const struct its_quirk its_quirks[] = {
> }
> };
>
> -static const struct its_quirk *__init gicv3_its_find_quirk(uint32_t iidr)
> +static const struct its_quirk *__init gicv3_its_find_quirk(
> + const struct host_its *hw_its)
> {
> - const struct its_quirk *quirks = its_quirks;
> + const struct its_quirk *quirk;
Is this change really required?
>
> /*
> - * The first matching quirk wins. More specific quirks must be listed
> - * before broader IIDR-only entries.
> + * The first matching quirk wins. Entries that match a specific platform
> + * must be listed before broader IIDR-only entries.
> */
> - for ( ; quirks->desc; quirks++ )
> + for ( quirk = its_quirks; quirk->desc; quirk++ )
> {
> - if ( quirks->iidr == (quirks->mask & iidr) )
> - return quirks;
> + ASSERT(quirk->match);
> +
> + if ( quirk->match && quirk->match(hw_its, quirk->data) )
> + return quirk;
> }
>
> return NULL;
> @@ -97,8 +141,7 @@ static const struct its_quirk *__init gicv3_its_find_quirk(uint32_t iidr)
>
> static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
> {
> - uint32_t iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
> - const struct its_quirk *quirk = gicv3_its_find_quirk(iidr);
> + const struct its_quirk *quirk = gicv3_its_find_quirk(hw_its);
>
> if ( quirk )
> {
--
WBR, Volodymyr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes
2026-09-22 15:58 ` [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes Mykola Kvach
@ 2026-09-23 0:29 ` Volodymyr Babchuk
2026-09-24 6:48 ` Mykola Kvach
0 siblings, 1 reply; 16+ messages in thread
From: Volodymyr Babchuk @ 2026-09-23 0:29 UTC (permalink / raw)
To: Mykola Kvach
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel
Hi,
Mykola Kvach <mykola_kvach@epam.com> writes:
> The DT dma-noncoherent property describes the bus coherency of the
> device represented by the node. On an ITS subnode, that is memory
> accessed by that ITS. Add GICV3_QUIRK_MEM_NC_NS to the corresponding
> host_its and use it when programming GITS_CBASER and GITS_BASER<n>.
>
> On the top-level GIC node, the property describes the Redistributor side
> of the LPI path. Collect it in gicv3_lpi_init_host_lpis() and apply it
> only to the host LPI policy used when allocating the property and
> pending tables and when programming GICR_PROPBASER and GICR_PENDBASER.
> Mark the function init-only because firmware attributes are collected
> only during boot.
>
> Do not inherit the property between parent and child nodes: ITS-node
> non-coherency does not change the global host LPI policy, and GIC-node
> non-coherency does not change per-ITS quirk_flags.
>
> ACPI is unchanged; this patch only consumes the DT dma-noncoherent
> property.
>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> Changes in v3:
> - Keep the ITS init annotations with the earlier initialization patch.
> - Drop the redundant aggregate host-LPI-flags message.
>
> Changes in v2:
> - Split v1's dma-noncoherent handling into explicit ITS-node and GIC-node
> scopes.
> - Apply an ITS subnode property only to the matching host_its
> quirk_flags.
> - Collect the top-level GIC property from gic-v3-lpi.c before host LPI
> allocations use host_lpi_flags.
> ---
> xen/arch/arm/gic-v3-its.c | 17 +++++++++++++++++
> xen/arch/arm/gic-v3-lpi.c | 20 +++++++++++++++++++-
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index f52232ca40..6734f94e7c 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -139,6 +139,21 @@ static const struct its_quirk *__init gicv3_its_find_quirk(
> return NULL;
> }
>
> +static void __init gicv3_its_collect_fw_attrs(struct host_its *hw_its)
> +{
> + /*
> + * An ITS subnode property describes memory transactions made by that ITS.
> + * Do not inherit it into the global host LPI/Redistributor policy.
> + */
> + if ( !hw_its->dt_node ||
> + !dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
> + return;
> +
> + hw_its->quirk_flags |= GICV3_QUIRK_MEM_NC_NS;
> + printk("GICv3: ITS @%#"PRIpaddr" marked dma-noncoherent\n",
> + hw_its->addr);
> +}
> +
> static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
> {
> const struct its_quirk *quirk = gicv3_its_find_quirk(hw_its);
> @@ -149,6 +164,8 @@ static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
> gicv3_lpi_update_host_flags(quirk->lpi_flags);
> printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
> }
> +
> + gicv3_its_collect_fw_attrs(hw_its);
> }
>
> uint64_t gicv3_mem_get_cacheability(uint32_t flags)
> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> index accfd48a74..1cf49430ea 100644
> --- a/xen/arch/arm/gic-v3-lpi.c
> +++ b/xen/arch/arm/gic-v3-lpi.c
> @@ -7,7 +7,9 @@
> * Copyright (C) 2016,2017 - ARM Ltd
> */
>
> +#include <xen/acpi.h>
> #include <xen/cpu.h>
> +#include <xen/device_tree.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/mm.h>
> @@ -102,6 +104,20 @@ void __init gicv3_lpi_update_host_flags(uint32_t flags)
> host_lpi_flags |= flags;
> }
>
> +static void __init gicv3_lpi_collect_fw_attrs(void)
> +{
> + /*
> + * A top-level GIC node property describes the Redistributor side of the
> + * LPI path. Do not inherit it into per-ITS policy.
> + */
> + if ( !acpi_disabled ||
> + !dt_property_read_bool(dt_interrupt_controller, "dma-noncoherent") )
> + return;
> +
> + gicv3_lpi_update_host_flags(GICV3_QUIRK_MEM_NC_NS);
> + printk("GICv3: Redistributors marked dma-noncoherent\n");
> +}
> +
> static union host_lpi *gic_get_host_lpi(uint32_t plpi)
> {
> union host_lpi *block;
> @@ -443,7 +459,7 @@ integer_param("max_lpi_bits", max_lpi_bits);
> * to the page with the actual "union host_lpi" entries. Our LPI limit
> * avoids excessive memory usage.
> */
> -int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
> +int __init gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
Spurious change?
> {
> unsigned int nr_lpi_ptrs;
> int rc;
> @@ -451,6 +467,8 @@ int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
> /* We rely on the data structure being atomically accessible. */
> BUILD_BUG_ON(sizeof(union host_lpi) > sizeof(unsigned long));
>
> + gicv3_lpi_collect_fw_attrs();
> +
> /*
> * An implementation needs to support at least 14 bits of LPI IDs.
> * Tell the user about it, the actual number is reported below.
--
WBR, Volodymyr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] xen/arm: its: refactor ITS quirk matching
2026-09-23 0:21 ` Volodymyr Babchuk
@ 2026-09-24 5:00 ` Mykola Kvach
2026-09-25 17:40 ` Volodymyr Babchuk
0 siblings, 1 reply; 16+ messages in thread
From: Mykola Kvach @ 2026-09-24 5:00 UTC (permalink / raw)
To: Volodymyr Babchuk
Cc: Mykola Kvach, xen-devel@lists.xenproject.org, Stefano Stabellini,
Julien Grall, Bertrand Marquis, Michal Orzel
On Wed, Sep 23, 2026 at 3:21 AM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> Hi,
>
> Mykola Kvach <mykola_kvach@epam.com> writes:
>
> > ITS quirks are matched only by IIDR and mask fields stored in each table
> > entry. That is too coarse when the same GIC IP block appears on several
> > platforms but a workaround is valid only for some of them.
> >
> > Replace the fixed IIDR fields with a generic match(hw_its, data)
> > callback and an opaque data pointer. Add an IIDR matcher as a reusable
> > building block and use it from the R-Car Gen4 matcher after checking the
> > Renesas machine compatibles.
> >
> > This intentionally narrows the R-Car Gen4 quirk. Previously every ITS
> > with IIDR 0x0201743b matched. Now it matches only a DT-discovered ITS on
> > an r8a779f0 or r8a779g0 machine. ACPI-discovered ITSes and the same IIDR
> > on other platforms no longer match.
> >
> > Keep first-match semantics explicit. Assert that non-sentinel entries
> > provide a matcher and that IIDR matching receives match data. Retain
> > runtime guards so malformed entries cannot cause a NULL function call
> > or data dereference in non-debug builds. Place the matcher data and
> > table in init-only read-only sections.
> >
> > The matched entry still supplies separate ITS and LPI flags; this patch
> > only changes how the entry is selected.
> >
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > Changes in v3:
> > - Document the intentional narrowing of the R-Car Gen4 match.
> > - Add the non-debug NULL-data guard.
> > - Put the matcher data and table in init-only read-only sections.
> >
> > Changes in v2:
> > - Replace v1's optional platform callback plus fixed IIDR/mask fields
> > with a single generic match(hw_its, data) selector.
> > - Add a reusable IIDR matcher and use it after the R-Car Gen4
> > machine-compatible checks.
> > - Document that the R-Car Gen4 quirk remains DT-only.
> > - Keep the split ITS and host LPI quirk scopes when applying the matched
> > entry.
> > - Document first-match ordering in the lookup path and guard against
> > entries without a match callback or IIDR match data.
> > ---
> > xen/arch/arm/gic-v3-its.c | 73 +++++++++++++++++++++++++++++++--------
> > 1 file changed, 58 insertions(+), 15 deletions(-)
> >
> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> > index 454fd0e0ef..f52232ca40 100644
> > --- a/xen/arch/arm/gic-v3-its.c
> > +++ b/xen/arch/arm/gic-v3-its.c
> > @@ -54,8 +54,8 @@ struct its_device {
> >
> > struct its_quirk {
> > const char *desc;
> > - uint32_t iidr;
> > - uint32_t mask;
> > + bool (*match)(const struct host_its *hw_its, const void *data);
> > + const void *data;
> > uint32_t its_flags;
> > /*
> > * lpi_flags are ORed into the global host LPI policy and must only
> > @@ -65,11 +65,52 @@ struct its_quirk {
> > uint32_t lpi_flags;
> > };
> >
> > -static const struct its_quirk its_quirks[] = {
> > +struct its_quirk_match_iidr {
> > + uint32_t iidr;
> > + uint32_t mask;
> > +};
> > +
> > +static bool __init gicv3_its_match_iidr(const struct host_its *hw_its,
> > + const void *data)
> > +{
> > + const struct its_quirk_match_iidr *match;
> > + uint32_t iidr;
> > +
> > + ASSERT(data);
> > +
> > + if ( !data )
> > + return false;
>
> It looks strange to have both ASSERT and then this check.
>
> Maybe it is worth to remove ASSERT and print a warning in a debug build instead?
The ASSERT catches a programming error in the static quirk table in
debug builds, while the explicit NULL check prevents a dereference
when assertions are disabled.
However, returning false skips a potentially required workaround,
which could cause a less obvious failure later. This also raises the
question of whether continuing is appropriate in non-debug builds.
Would you prefer the debug warning you suggested, keeping ASSERT
with an additional warning in non-debug builds, or using BUG_ON(!data)
to stop immediately in all builds?
>
> > +
> > + match = data;
> > + iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
> > +
> > + return (iidr & match->mask) == match->iidr;
> > +}
> > +
> > +static bool __init gicv3_its_match_quirk_gen4(const struct host_its *hw_its,
> > + const void *data)
> > +{
> > + if ( !hw_its->dt_node )
> > + return false;
> > +
> > + if ( !dt_machine_is_compatible("renesas,r8a779f0") &&
> > + !dt_machine_is_compatible("renesas,r8a779g0") )
> > + return false;
> > +
> > + return gicv3_its_match_iidr(hw_its, data);
>
> Do you really need to match IIDR in this case? You already know that
> platform requires the quirk.
I would prefer to retain the IIDR check. The machine compatible does
not identify the exact ITS variant and revision. Keeping both checks
preserves the existing IIDR restriction while narrowing the match to
the affected platforms.
Linux also uses both checks for the R-Car Gen4 DMA32 workaround:
the quirk entry matches IIDR 0x0201743b with mask 0xffffffff, and
its_enable_dma32() additionally checks the machine compatible.
>
> > +}
> > +
> > +static const struct its_quirk_match_iidr rcar_gen4_iidr __initconst = {
> > + /* Implementer 0x43b identifies Arm Ltd. */
> > + .iidr = 0x0201743b,
> > + .mask = 0xffffffffU,
> > +};
> > +
> > +static const struct its_quirk its_quirks[] __initconstrel = {
> > {
> > - .desc = "R-Car Gen4",
> > - .iidr = 0x0201743b,
> > - .mask = 0xffffffffU,
> > + .desc = "R-Car Gen4",
> > + .match = gicv3_its_match_quirk_gen4,
> > + .data = &rcar_gen4_iidr,
> > .its_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
> > .lpi_flags = GICV3_QUIRK_MEM_NC_NS | GICV3_QUIRK_MEM_32BIT_ADDR,
> > },
> > @@ -78,18 +119,21 @@ static const struct its_quirk its_quirks[] = {
> > }
> > };
> >
> > -static const struct its_quirk *__init gicv3_its_find_quirk(uint32_t iidr)
> > +static const struct its_quirk *__init gicv3_its_find_quirk(
> > + const struct host_its *hw_its)
> > {
> > - const struct its_quirk *quirks = its_quirks;
> > + const struct its_quirk *quirk;
>
> Is this change really required?
Agreed, the rename is unnecessary. A single entry can carry several
quirk flags, so the existing name also makes sense. I'll keep quirks.
Best regards,
Mykola
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes
2026-09-22 23:56 ` Volodymyr Babchuk
@ 2026-09-24 5:00 ` Mykola Kvach
2026-09-25 17:45 ` Volodymyr Babchuk
0 siblings, 1 reply; 16+ messages in thread
From: Mykola Kvach @ 2026-09-24 5:00 UTC (permalink / raw)
To: Volodymyr Babchuk
Cc: Mykola Kvach, xen-devel@lists.xenproject.org, Stefano Stabellini,
Julien Grall, Bertrand Marquis, Michal Orzel
Hi Volodymyr,
Thank you for the review.
On Wed, Sep 23, 2026 at 2:56 AM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> Hi Mykola,
>
> Mykola Kvach <mykola_kvach@epam.com> writes:
>
> > The boot CPU pending table must use the memory attributes selected by
> > ITS quirks. gicv3_lpi_init_host_lpis() is therefore called after
> > gicv3_its_init(). However, gicv3_its_init() also programs and enables
> > each ITS before host LPI state is allocated. No ITS commands are
> > submitted at that point, but this ordering relies on that implementation
> > detail.
> >
> > Split per-ITS initialization into preparation and activation phases.
> > First map and disable every ITS and collect its quirks. Then initialize
> > host LPI state. Only after that, allocate and program the ITS tables and
> > command queue, and enable each ITS.
> >
> > The subsequent gicv3_cpu_init() sequence remains unchanged: it programs
> > the Redistributor LPI tables, enables LPIs, and then submits the first
> > MAPC and SYNC commands.
> >
> > Suggested-by: Julien Grall <julien@xen.org>
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > Changes in v3:
> > - New patch implementing the post-4.22 initialization order discussed
> > during review of the ordering fix.
> >
> > Link: https://patchew.org/Xen/341edd8de63dcd84ccc6e7b6c03e9e8fc7105184.1781847061.git.mykola._5Fkvach@epam.com/
> > ---
> > xen/arch/arm/gic-v3-its.c | 32 ++++++++++++++++++++++-----
> > xen/arch/arm/gic-v3.c | 15 ++-----------
> > xen/arch/arm/include/asm/gic_v3_its.h | 4 ++--
> > 3 files changed, 31 insertions(+), 20 deletions(-)
> >
> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> > index 325835b0ad..972825bf06 100644
> > --- a/xen/arch/arm/gic-v3-its.c
> > +++ b/xen/arch/arm/gic-v3-its.c
> > @@ -11,6 +11,7 @@
> > #include <xen/lib.h>
> > #include <xen/delay.h>
> > #include <xen/iocap.h>
> > +#include <xen/init.h>
> > #include <xen/libfdt/libfdt.h>
> > #include <xen/mm.h>
> > #include <xen/rbtree.h>
> > @@ -549,10 +550,9 @@ static int gicv3_disable_its(struct host_its *hw_its)
> > return -ETIMEDOUT;
> > }
> >
> > -static int gicv3_its_init_single_its(struct host_its *hw_its)
> > +static int __init gicv3_its_prepare_single_its(struct host_its *hw_its)
> > {
> > - uint64_t reg;
> > - int i, ret;
> > + int ret;
> >
> > hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
> > if ( !hw_its->its_base )
> > @@ -564,6 +564,14 @@ static int gicv3_its_init_single_its(struct host_its *hw_its)
> >
> > gicv3_its_enable_quirks(hw_its);
> >
> > + return 0;
> > +}
> > +
> > +static int __init gicv3_its_init_single_its(struct host_its *hw_its)
> > +{
> > + uint64_t reg;
> > + int i, ret;
> > +
> > 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);
> > @@ -1189,7 +1197,7 @@ static void gicv3_its_acpi_init(void)
> >
> > #endif
> >
> > -int gicv3_its_init(void)
> > +int __init gicv3_its_init(unsigned int host_lpi_bits)
> > {
> > struct host_its *hw_its;
> > int ret;
> > @@ -1201,13 +1209,27 @@ int gicv3_its_init(void)
> >
> > list_for_each_entry(hw_its, &host_its_list, entry)
> > {
> > - ret = gicv3_its_init_single_its(hw_its);
> > + ret = gicv3_its_prepare_single_its(hw_its);
> > if ( ret )
> > return ret;
> > }
> >
> > gicv3_its_validate_quirks();
> >
> > + if ( list_empty(&host_its_list) )
> > + return 0;
>
> What is the purpose of this check here? I'd expect to see it before the
> first list_for_each_entry() loop.
The check skips host LPI initialization when no host ITS is present,
preserving the existing behavior. Xen currently does not support LPIs
without an ITS.
gicv3_its_validate_quirks() already checks for an empty list, so the
current placement does not cause a functional issue. The following
patch also removes that function and its call.
Would you be OK with keeping the current placement?
Best regards,
Mykola
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes
2026-09-23 0:29 ` Volodymyr Babchuk
@ 2026-09-24 6:48 ` Mykola Kvach
2026-09-25 17:46 ` Volodymyr Babchuk
0 siblings, 1 reply; 16+ messages in thread
From: Mykola Kvach @ 2026-09-24 6:48 UTC (permalink / raw)
To: Volodymyr Babchuk
Cc: Mykola Kvach, xen-devel@lists.xenproject.org, Stefano Stabellini,
Julien Grall, Bertrand Marquis, Michal Orzel
On Wed, Sep 23, 2026 at 3:30 AM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> Hi,
>
> Mykola Kvach <mykola_kvach@epam.com> writes:
>
> > The DT dma-noncoherent property describes the bus coherency of the
> > device represented by the node. On an ITS subnode, that is memory
> > accessed by that ITS. Add GICV3_QUIRK_MEM_NC_NS to the corresponding
> > host_its and use it when programming GITS_CBASER and GITS_BASER<n>.
> >
> > On the top-level GIC node, the property describes the Redistributor side
> > of the LPI path. Collect it in gicv3_lpi_init_host_lpis() and apply it
> > only to the host LPI policy used when allocating the property and
> > pending tables and when programming GICR_PROPBASER and GICR_PENDBASER.
> > Mark the function init-only because firmware attributes are collected
> > only during boot.
> >
> > Do not inherit the property between parent and child nodes: ITS-node
> > non-coherency does not change the global host LPI policy, and GIC-node
> > non-coherency does not change per-ITS quirk_flags.
> >
> > ACPI is unchanged; this patch only consumes the DT dma-noncoherent
> > property.
> >
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > Changes in v3:
> > - Keep the ITS init annotations with the earlier initialization patch.
> > - Drop the redundant aggregate host-LPI-flags message.
> >
> > Changes in v2:
> > - Split v1's dma-noncoherent handling into explicit ITS-node and GIC-node
> > scopes.
> > - Apply an ITS subnode property only to the matching host_its
> > quirk_flags.
> > - Collect the top-level GIC property from gic-v3-lpi.c before host LPI
> > allocations use host_lpi_flags.
> > ---
> > xen/arch/arm/gic-v3-its.c | 17 +++++++++++++++++
> > xen/arch/arm/gic-v3-lpi.c | 20 +++++++++++++++++++-
> > 2 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> > index f52232ca40..6734f94e7c 100644
> > --- a/xen/arch/arm/gic-v3-its.c
> > +++ b/xen/arch/arm/gic-v3-its.c
> > @@ -139,6 +139,21 @@ static const struct its_quirk *__init gicv3_its_find_quirk(
> > return NULL;
> > }
> >
> > +static void __init gicv3_its_collect_fw_attrs(struct host_its *hw_its)
> > +{
> > + /*
> > + * An ITS subnode property describes memory transactions made by that ITS.
> > + * Do not inherit it into the global host LPI/Redistributor policy.
> > + */
> > + if ( !hw_its->dt_node ||
> > + !dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
> > + return;
> > +
> > + hw_its->quirk_flags |= GICV3_QUIRK_MEM_NC_NS;
> > + printk("GICv3: ITS @%#"PRIpaddr" marked dma-noncoherent\n",
> > + hw_its->addr);
> > +}
> > +
> > static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
> > {
> > const struct its_quirk *quirk = gicv3_its_find_quirk(hw_its);
> > @@ -149,6 +164,8 @@ static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
> > gicv3_lpi_update_host_flags(quirk->lpi_flags);
> > printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
> > }
> > +
> > + gicv3_its_collect_fw_attrs(hw_its);
> > }
> >
> > uint64_t gicv3_mem_get_cacheability(uint32_t flags)
> > diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> > index accfd48a74..1cf49430ea 100644
> > --- a/xen/arch/arm/gic-v3-lpi.c
> > +++ b/xen/arch/arm/gic-v3-lpi.c
> > @@ -7,7 +7,9 @@
> > * Copyright (C) 2016,2017 - ARM Ltd
> > */
> >
> > +#include <xen/acpi.h>
> > #include <xen/cpu.h>
> > +#include <xen/device_tree.h>
> > #include <xen/init.h>
> > #include <xen/lib.h>
> > #include <xen/mm.h>
> > @@ -102,6 +104,20 @@ void __init gicv3_lpi_update_host_flags(uint32_t flags)
> > host_lpi_flags |= flags;
> > }
> >
> > +static void __init gicv3_lpi_collect_fw_attrs(void)
> > +{
> > + /*
> > + * A top-level GIC node property describes the Redistributor side of the
> > + * LPI path. Do not inherit it into per-ITS policy.
> > + */
> > + if ( !acpi_disabled ||
> > + !dt_property_read_bool(dt_interrupt_controller, "dma-noncoherent") )
> > + return;
> > +
> > + gicv3_lpi_update_host_flags(GICV3_QUIRK_MEM_NC_NS);
> > + printk("GICv3: Redistributors marked dma-noncoherent\n");
> > +}
> > +
> > static union host_lpi *gic_get_host_lpi(uint32_t plpi)
> > {
> > union host_lpi *block;
> > @@ -443,7 +459,7 @@ integer_param("max_lpi_bits", max_lpi_bits);
> > * to the page with the actual "union host_lpi" entries. Our LPI limit
> > * avoids excessive memory usage.
> > */
> > -int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
> > +int __init gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
>
> Spurious change?
This is intentional. This patch adds a call to the __init helper
gicv3_lpi_collect_fw_attrs(). gicv3_lpi_init_host_lpis() is only
called during boot, from gicv3_its_init(), so I marked it __init
as well.
Best regards,
Mykola
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] xen/arm: its: refactor ITS quirk matching
2026-09-24 5:00 ` Mykola Kvach
@ 2026-09-25 17:40 ` Volodymyr Babchuk
0 siblings, 0 replies; 16+ messages in thread
From: Volodymyr Babchuk @ 2026-09-25 17:40 UTC (permalink / raw)
To: Mykola Kvach
Cc: Mykola Kvach, xen-devel@lists.xenproject.org, Stefano Stabellini,
Julien Grall, Bertrand Marquis, Michal Orzel
Hi Mykola,
Mykola Kvach <xakep.amatop@gmail.com> writes:
> On Wed, Sep 23, 2026 at 3:21 AM Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com> wrote:
>>
>> Hi,
>>
>> Mykola Kvach <mykola_kvach@epam.com> writes:
>>
>> > ITS quirks are matched only by IIDR and mask fields stored in each table
>> > entry. That is too coarse when the same GIC IP block appears on several
>> > platforms but a workaround is valid only for some of them.
>> >
>> > Replace the fixed IIDR fields with a generic match(hw_its, data)
>> > callback and an opaque data pointer. Add an IIDR matcher as a reusable
>> > building block and use it from the R-Car Gen4 matcher after checking the
>> > Renesas machine compatibles.
>> >
>> > This intentionally narrows the R-Car Gen4 quirk. Previously every ITS
>> > with IIDR 0x0201743b matched. Now it matches only a DT-discovered ITS on
>> > an r8a779f0 or r8a779g0 machine. ACPI-discovered ITSes and the same IIDR
>> > on other platforms no longer match.
>> >
>> > Keep first-match semantics explicit. Assert that non-sentinel entries
>> > provide a matcher and that IIDR matching receives match data. Retain
>> > runtime guards so malformed entries cannot cause a NULL function call
>> > or data dereference in non-debug builds. Place the matcher data and
>> > table in init-only read-only sections.
>> >
>> > The matched entry still supplies separate ITS and LPI flags; this patch
>> > only changes how the entry is selected.
>> >
>> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>> > ---
>> > Changes in v3:
>> > - Document the intentional narrowing of the R-Car Gen4 match.
>> > - Add the non-debug NULL-data guard.
>> > - Put the matcher data and table in init-only read-only sections.
>> >
>> > Changes in v2:
>> > - Replace v1's optional platform callback plus fixed IIDR/mask fields
>> > with a single generic match(hw_its, data) selector.
>> > - Add a reusable IIDR matcher and use it after the R-Car Gen4
>> > machine-compatible checks.
>> > - Document that the R-Car Gen4 quirk remains DT-only.
>> > - Keep the split ITS and host LPI quirk scopes when applying the matched
>> > entry.
>> > - Document first-match ordering in the lookup path and guard against
>> > entries without a match callback or IIDR match data.
>> > ---
>> > xen/arch/arm/gic-v3-its.c | 73 +++++++++++++++++++++++++++++++--------
>> > 1 file changed, 58 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> > index 454fd0e0ef..f52232ca40 100644
>> > --- a/xen/arch/arm/gic-v3-its.c
>> > +++ b/xen/arch/arm/gic-v3-its.c
>> > @@ -54,8 +54,8 @@ struct its_device {
>> >
>> > struct its_quirk {
>> > const char *desc;
>> > - uint32_t iidr;
>> > - uint32_t mask;
>> > + bool (*match)(const struct host_its *hw_its, const void *data);
>> > + const void *data;
>> > uint32_t its_flags;
>> > /*
>> > * lpi_flags are ORed into the global host LPI policy and must only
>> > @@ -65,11 +65,52 @@ struct its_quirk {
>> > uint32_t lpi_flags;
>> > };
>> >
>> > -static const struct its_quirk its_quirks[] = {
>> > +struct its_quirk_match_iidr {
>> > + uint32_t iidr;
>> > + uint32_t mask;
>> > +};
>> > +
>> > +static bool __init gicv3_its_match_iidr(const struct host_its *hw_its,
>> > + const void *data)
>> > +{
>> > + const struct its_quirk_match_iidr *match;
>> > + uint32_t iidr;
>> > +
>> > + ASSERT(data);
>> > +
>> > + if ( !data )
>> > + return false;
>>
>> It looks strange to have both ASSERT and then this check.
>>
>> Maybe it is worth to remove ASSERT and print a warning in a debug build instead?
>
> The ASSERT catches a programming error in the static quirk table in
> debug builds, while the explicit NULL check prevents a dereference
> when assertions are disabled.
>
> However, returning false skips a potentially required workaround,
> which could cause a less obvious failure later. This also raises the
> question of whether continuing is appropriate in non-debug builds.
>
> Would you prefer the debug warning you suggested, keeping ASSERT
> with an additional warning in non-debug builds, or using BUG_ON(!data)
> to stop immediately in all builds?
Thinking about this... Yes, I believe that should be BUG_ON()
>
>>
>> > +
>> > + match = data;
>> > + iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
>> > +
>> > + return (iidr & match->mask) == match->iidr;
>> > +}
>> > +
>> > +static bool __init gicv3_its_match_quirk_gen4(const struct host_its *hw_its,
>> > + const void *data)
>> > +{
>> > + if ( !hw_its->dt_node )
>> > + return false;
>> > +
>> > + if ( !dt_machine_is_compatible("renesas,r8a779f0") &&
>> > + !dt_machine_is_compatible("renesas,r8a779g0") )
>> > + return false;
>> > +
>> > + return gicv3_its_match_iidr(hw_its, data);
>>
>> Do you really need to match IIDR in this case? You already know that
>> platform requires the quirk.
>
> I would prefer to retain the IIDR check. The machine compatible does
> not identify the exact ITS variant and revision. Keeping both checks
> preserves the existing IIDR restriction while narrowing the match to
> the affected platforms.
>
> Linux also uses both checks for the R-Car Gen4 DMA32 workaround:
> the quirk entry matches IIDR 0x0201743b with mask 0xffffffff, and
> its_enable_dma32() additionally checks the machine compatible.
Okay then.
[...]
--
WBR, Volodymyr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes
2026-09-24 5:00 ` Mykola Kvach
@ 2026-09-25 17:45 ` Volodymyr Babchuk
2026-09-29 17:25 ` Mykola Kvach
0 siblings, 1 reply; 16+ messages in thread
From: Volodymyr Babchuk @ 2026-09-25 17:45 UTC (permalink / raw)
To: Mykola Kvach
Cc: Mykola Kvach, xen-devel@lists.xenproject.org, Stefano Stabellini,
Julien Grall, Bertrand Marquis, Michal Orzel
Hi Mykola,
Mykola Kvach <xakep.amatop@gmail.com> writes:
> Hi Volodymyr,
>
> Thank you for the review.
>
> On Wed, Sep 23, 2026 at 2:56 AM Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com> wrote:
>>
>> Hi Mykola,
>>
>> Mykola Kvach <mykola_kvach@epam.com> writes:
>>
>> > The boot CPU pending table must use the memory attributes selected by
>> > ITS quirks. gicv3_lpi_init_host_lpis() is therefore called after
>> > gicv3_its_init(). However, gicv3_its_init() also programs and enables
>> > each ITS before host LPI state is allocated. No ITS commands are
>> > submitted at that point, but this ordering relies on that implementation
>> > detail.
>> >
>> > Split per-ITS initialization into preparation and activation phases.
>> > First map and disable every ITS and collect its quirks. Then initialize
>> > host LPI state. Only after that, allocate and program the ITS tables and
>> > command queue, and enable each ITS.
>> >
>> > The subsequent gicv3_cpu_init() sequence remains unchanged: it programs
>> > the Redistributor LPI tables, enables LPIs, and then submits the first
>> > MAPC and SYNC commands.
>> >
>> > Suggested-by: Julien Grall <julien@xen.org>
>> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>> > ---
>> > Changes in v3:
>> > - New patch implementing the post-4.22 initialization order discussed
>> > during review of the ordering fix.
>> >
>> > Link: https://patchew.org/Xen/341edd8de63dcd84ccc6e7b6c03e9e8fc7105184.1781847061.git.mykola._5Fkvach@epam.com/
>> > ---
>> > xen/arch/arm/gic-v3-its.c | 32 ++++++++++++++++++++++-----
>> > xen/arch/arm/gic-v3.c | 15 ++-----------
>> > xen/arch/arm/include/asm/gic_v3_its.h | 4 ++--
>> > 3 files changed, 31 insertions(+), 20 deletions(-)
>> >
>> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> > index 325835b0ad..972825bf06 100644
>> > --- a/xen/arch/arm/gic-v3-its.c
>> > +++ b/xen/arch/arm/gic-v3-its.c
>> > @@ -11,6 +11,7 @@
>> > #include <xen/lib.h>
>> > #include <xen/delay.h>
>> > #include <xen/iocap.h>
>> > +#include <xen/init.h>
>> > #include <xen/libfdt/libfdt.h>
>> > #include <xen/mm.h>
>> > #include <xen/rbtree.h>
>> > @@ -549,10 +550,9 @@ static int gicv3_disable_its(struct host_its *hw_its)
>> > return -ETIMEDOUT;
>> > }
>> >
>> > -static int gicv3_its_init_single_its(struct host_its *hw_its)
>> > +static int __init gicv3_its_prepare_single_its(struct host_its *hw_its)
>> > {
>> > - uint64_t reg;
>> > - int i, ret;
>> > + int ret;
>> >
>> > hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
>> > if ( !hw_its->its_base )
>> > @@ -564,6 +564,14 @@ static int gicv3_its_init_single_its(struct host_its *hw_its)
>> >
>> > gicv3_its_enable_quirks(hw_its);
>> >
>> > + return 0;
>> > +}
>> > +
>> > +static int __init gicv3_its_init_single_its(struct host_its *hw_its)
>> > +{
>> > + uint64_t reg;
>> > + int i, ret;
>> > +
>> > 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);
>> > @@ -1189,7 +1197,7 @@ static void gicv3_its_acpi_init(void)
>> >
>> > #endif
>> >
>> > -int gicv3_its_init(void)
>> > +int __init gicv3_its_init(unsigned int host_lpi_bits)
>> > {
>> > struct host_its *hw_its;
>> > int ret;
>> > @@ -1201,13 +1209,27 @@ int gicv3_its_init(void)
>> >
>> > list_for_each_entry(hw_its, &host_its_list, entry)
>> > {
>> > - ret = gicv3_its_init_single_its(hw_its);
>> > + ret = gicv3_its_prepare_single_its(hw_its);
>> > if ( ret )
>> > return ret;
>> > }
>> >
>> > gicv3_its_validate_quirks();
>> >
>> > + if ( list_empty(&host_its_list) )
>> > + return 0;
>>
>> What is the purpose of this check here? I'd expect to see it before the
>> first list_for_each_entry() loop.
>
> The check skips host LPI initialization when no host ITS is present,
> preserving the existing behavior. Xen currently does not support LPIs
> without an ITS.
This does not answer why you need to run list_for_each_entry() loop. I
understand that it will not loop and then you will just call
gicv3_its_validate_quirks(), which also will do nothing. This is why I
am asking, why not move this if() check right before the loop?
> gicv3_its_validate_quirks() already checks for an empty list, so the
> current placement does not cause a functional issue. The following
> patch also removes that function and its call.
>
> Would you be OK with keeping the current placement?
I think it will be more sensible to have this check before the
loop. Unless I miss something, of course.
--
WBR, Volodymyr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes
2026-09-24 6:48 ` Mykola Kvach
@ 2026-09-25 17:46 ` Volodymyr Babchuk
0 siblings, 0 replies; 16+ messages in thread
From: Volodymyr Babchuk @ 2026-09-25 17:46 UTC (permalink / raw)
To: Mykola Kvach
Cc: Mykola Kvach, xen-devel@lists.xenproject.org, Stefano Stabellini,
Julien Grall, Bertrand Marquis, Michal Orzel
Hi
Mykola Kvach <xakep.amatop@gmail.com> writes:
> On Wed, Sep 23, 2026 at 3:30 AM Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com> wrote:
>>
>> Hi,
>>
>> Mykola Kvach <mykola_kvach@epam.com> writes:
>>
>> > The DT dma-noncoherent property describes the bus coherency of the
>> > device represented by the node. On an ITS subnode, that is memory
>> > accessed by that ITS. Add GICV3_QUIRK_MEM_NC_NS to the corresponding
>> > host_its and use it when programming GITS_CBASER and GITS_BASER<n>.
>> >
>> > On the top-level GIC node, the property describes the Redistributor side
>> > of the LPI path. Collect it in gicv3_lpi_init_host_lpis() and apply it
>> > only to the host LPI policy used when allocating the property and
>> > pending tables and when programming GICR_PROPBASER and GICR_PENDBASER.
>> > Mark the function init-only because firmware attributes are collected
>> > only during boot.
>> >
>> > Do not inherit the property between parent and child nodes: ITS-node
>> > non-coherency does not change the global host LPI policy, and GIC-node
>> > non-coherency does not change per-ITS quirk_flags.
>> >
>> > ACPI is unchanged; this patch only consumes the DT dma-noncoherent
>> > property.
>> >
>> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> > ---
>> > Changes in v3:
>> > - Keep the ITS init annotations with the earlier initialization patch.
>> > - Drop the redundant aggregate host-LPI-flags message.
>> >
>> > Changes in v2:
>> > - Split v1's dma-noncoherent handling into explicit ITS-node and GIC-node
>> > scopes.
>> > - Apply an ITS subnode property only to the matching host_its
>> > quirk_flags.
>> > - Collect the top-level GIC property from gic-v3-lpi.c before host LPI
>> > allocations use host_lpi_flags.
>> > ---
>> > xen/arch/arm/gic-v3-its.c | 17 +++++++++++++++++
>> > xen/arch/arm/gic-v3-lpi.c | 20 +++++++++++++++++++-
>> > 2 files changed, 36 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> > index f52232ca40..6734f94e7c 100644
>> > --- a/xen/arch/arm/gic-v3-its.c
>> > +++ b/xen/arch/arm/gic-v3-its.c
>> > @@ -139,6 +139,21 @@ static const struct its_quirk *__init gicv3_its_find_quirk(
>> > return NULL;
>> > }
>> >
>> > +static void __init gicv3_its_collect_fw_attrs(struct host_its *hw_its)
>> > +{
>> > + /*
>> > + * An ITS subnode property describes memory transactions made by that ITS.
>> > + * Do not inherit it into the global host LPI/Redistributor policy.
>> > + */
>> > + if ( !hw_its->dt_node ||
>> > + !dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
>> > + return;
>> > +
>> > + hw_its->quirk_flags |= GICV3_QUIRK_MEM_NC_NS;
>> > + printk("GICv3: ITS @%#"PRIpaddr" marked dma-noncoherent\n",
>> > + hw_its->addr);
>> > +}
>> > +
>> > static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
>> > {
>> > const struct its_quirk *quirk = gicv3_its_find_quirk(hw_its);
>> > @@ -149,6 +164,8 @@ static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
>> > gicv3_lpi_update_host_flags(quirk->lpi_flags);
>> > printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
>> > }
>> > +
>> > + gicv3_its_collect_fw_attrs(hw_its);
>> > }
>> >
>> > uint64_t gicv3_mem_get_cacheability(uint32_t flags)
>> > diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
>> > index accfd48a74..1cf49430ea 100644
>> > --- a/xen/arch/arm/gic-v3-lpi.c
>> > +++ b/xen/arch/arm/gic-v3-lpi.c
>> > @@ -7,7 +7,9 @@
>> > * Copyright (C) 2016,2017 - ARM Ltd
>> > */
>> >
>> > +#include <xen/acpi.h>
>> > #include <xen/cpu.h>
>> > +#include <xen/device_tree.h>
>> > #include <xen/init.h>
>> > #include <xen/lib.h>
>> > #include <xen/mm.h>
>> > @@ -102,6 +104,20 @@ void __init gicv3_lpi_update_host_flags(uint32_t flags)
>> > host_lpi_flags |= flags;
>> > }
>> >
>> > +static void __init gicv3_lpi_collect_fw_attrs(void)
>> > +{
>> > + /*
>> > + * A top-level GIC node property describes the Redistributor side of the
>> > + * LPI path. Do not inherit it into per-ITS policy.
>> > + */
>> > + if ( !acpi_disabled ||
>> > + !dt_property_read_bool(dt_interrupt_controller, "dma-noncoherent") )
>> > + return;
>> > +
>> > + gicv3_lpi_update_host_flags(GICV3_QUIRK_MEM_NC_NS);
>> > + printk("GICv3: Redistributors marked dma-noncoherent\n");
>> > +}
>> > +
>> > static union host_lpi *gic_get_host_lpi(uint32_t plpi)
>> > {
>> > union host_lpi *block;
>> > @@ -443,7 +459,7 @@ integer_param("max_lpi_bits", max_lpi_bits);
>> > * to the page with the actual "union host_lpi" entries. Our LPI limit
>> > * avoids excessive memory usage.
>> > */
>> > -int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
>> > +int __init gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
>>
>> Spurious change?
>
> This is intentional. This patch adds a call to the __init helper
> gicv3_lpi_collect_fw_attrs(). gicv3_lpi_init_host_lpis() is only
> called during boot, from gicv3_its_init(), so I marked it __init
> as well.
Makes sense, thank you.
--
WBR, Volodymyr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes
2026-09-25 17:45 ` Volodymyr Babchuk
@ 2026-09-29 17:25 ` Mykola Kvach
0 siblings, 0 replies; 16+ messages in thread
From: Mykola Kvach @ 2026-09-29 17:25 UTC (permalink / raw)
To: Volodymyr Babchuk
Cc: Mykola Kvach, xen-devel@lists.xenproject.org, Stefano Stabellini,
Julien Grall, Bertrand Marquis, Michal Orzel
On Fri, Sep 25, 2026 at 8:45 PM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> Hi Mykola,
>
> Mykola Kvach <xakep.amatop@gmail.com> writes:
>
> > Hi Volodymyr,
> >
> > Thank you for the review.
> >
> > On Wed, Sep 23, 2026 at 2:56 AM Volodymyr Babchuk
> > <Volodymyr_Babchuk@epam.com> wrote:
> >>
> >> Hi Mykola,
> >>
> >> Mykola Kvach <mykola_kvach@epam.com> writes:
> >>
> >> > The boot CPU pending table must use the memory attributes selected by
> >> > ITS quirks. gicv3_lpi_init_host_lpis() is therefore called after
> >> > gicv3_its_init(). However, gicv3_its_init() also programs and enables
> >> > each ITS before host LPI state is allocated. No ITS commands are
> >> > submitted at that point, but this ordering relies on that implementation
> >> > detail.
> >> >
> >> > Split per-ITS initialization into preparation and activation phases.
> >> > First map and disable every ITS and collect its quirks. Then initialize
> >> > host LPI state. Only after that, allocate and program the ITS tables and
> >> > command queue, and enable each ITS.
> >> >
> >> > The subsequent gicv3_cpu_init() sequence remains unchanged: it programs
> >> > the Redistributor LPI tables, enables LPIs, and then submits the first
> >> > MAPC and SYNC commands.
> >> >
> >> > Suggested-by: Julien Grall <julien@xen.org>
> >> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> >> > ---
> >> > Changes in v3:
> >> > - New patch implementing the post-4.22 initialization order discussed
> >> > during review of the ordering fix.
> >> >
> >> > Link: https://patchew.org/Xen/341edd8de63dcd84ccc6e7b6c03e9e8fc7105184.1781847061.git.mykola._5Fkvach@epam.com/
> >> > ---
> >> > xen/arch/arm/gic-v3-its.c | 32 ++++++++++++++++++++++-----
> >> > xen/arch/arm/gic-v3.c | 15 ++-----------
> >> > xen/arch/arm/include/asm/gic_v3_its.h | 4 ++--
> >> > 3 files changed, 31 insertions(+), 20 deletions(-)
> >> >
> >> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> >> > index 325835b0ad..972825bf06 100644
> >> > --- a/xen/arch/arm/gic-v3-its.c
> >> > +++ b/xen/arch/arm/gic-v3-its.c
> >> > @@ -11,6 +11,7 @@
> >> > #include <xen/lib.h>
> >> > #include <xen/delay.h>
> >> > #include <xen/iocap.h>
> >> > +#include <xen/init.h>
> >> > #include <xen/libfdt/libfdt.h>
> >> > #include <xen/mm.h>
> >> > #include <xen/rbtree.h>
> >> > @@ -549,10 +550,9 @@ static int gicv3_disable_its(struct host_its *hw_its)
> >> > return -ETIMEDOUT;
> >> > }
> >> >
> >> > -static int gicv3_its_init_single_its(struct host_its *hw_its)
> >> > +static int __init gicv3_its_prepare_single_its(struct host_its *hw_its)
> >> > {
> >> > - uint64_t reg;
> >> > - int i, ret;
> >> > + int ret;
> >> >
> >> > hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
> >> > if ( !hw_its->its_base )
> >> > @@ -564,6 +564,14 @@ static int gicv3_its_init_single_its(struct host_its *hw_its)
> >> >
> >> > gicv3_its_enable_quirks(hw_its);
> >> >
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static int __init gicv3_its_init_single_its(struct host_its *hw_its)
> >> > +{
> >> > + uint64_t reg;
> >> > + int i, ret;
> >> > +
> >> > 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);
> >> > @@ -1189,7 +1197,7 @@ static void gicv3_its_acpi_init(void)
> >> >
> >> > #endif
> >> >
> >> > -int gicv3_its_init(void)
> >> > +int __init gicv3_its_init(unsigned int host_lpi_bits)
> >> > {
> >> > struct host_its *hw_its;
> >> > int ret;
> >> > @@ -1201,13 +1209,27 @@ int gicv3_its_init(void)
> >> >
> >> > list_for_each_entry(hw_its, &host_its_list, entry)
> >> > {
> >> > - ret = gicv3_its_init_single_its(hw_its);
> >> > + ret = gicv3_its_prepare_single_its(hw_its);
> >> > if ( ret )
> >> > return ret;
> >> > }
> >> >
> >> > gicv3_its_validate_quirks();
> >> >
> >> > + if ( list_empty(&host_its_list) )
> >> > + return 0;
> >>
> >> What is the purpose of this check here? I'd expect to see it before the
> >> first list_for_each_entry() loop.
> >
> > The check skips host LPI initialization when no host ITS is present,
> > preserving the existing behavior. Xen currently does not support LPIs
> > without an ITS.
>
> This does not answer why you need to run list_for_each_entry() loop. I
> understand that it will not loop and then you will just call
> gicv3_its_validate_quirks(), which also will do nothing. This is why I
> am asking, why not move this if() check right before the loop?
>
> > gicv3_its_validate_quirks() already checks for an empty list, so the
> > current placement does not cause a functional issue. The following
> > patch also removes that function and its call.
> >
> > Would you be OK with keeping the current placement?
>
> I think it will be more sensible to have this check before the
> loop. Unless I miss something, of course.
Ack.
Best regards,
Mykola
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-09-29 17:26 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-22 15:58 [PATCH v3 0/4] xen/arm: gicv3: defer host LPI init and split ITS/LPI quirk scopes Mykola Kvach
2026-09-22 15:58 ` [PATCH v3 1/4] xen/arm: its: initialize host LPI state before activating ITSes Mykola Kvach
2026-09-22 23:56 ` Volodymyr Babchuk
2026-09-24 5:00 ` Mykola Kvach
2026-09-25 17:45 ` Volodymyr Babchuk
2026-09-29 17:25 ` Mykola Kvach
2026-09-22 15:58 ` [PATCH v3 2/4] xen/arm: its: separate ITS and host LPI quirk scopes Mykola Kvach
2026-09-23 0:13 ` Volodymyr Babchuk
2026-09-22 15:58 ` [PATCH v3 3/4] xen/arm: its: refactor ITS quirk matching Mykola Kvach
2026-09-23 0:21 ` Volodymyr Babchuk
2026-09-24 5:00 ` Mykola Kvach
2026-09-25 17:40 ` Volodymyr Babchuk
2026-09-22 15:58 ` [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes Mykola Kvach
2026-09-23 0:29 ` Volodymyr Babchuk
2026-09-24 6:48 ` Mykola Kvach
2026-09-25 17:46 ` Volodymyr Babchuk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.