From: Mykola Kvach <mykola_kvach@epam.com>
Replace the per-quirk init callback with declarative flags in
struct its_quirk, and introduce gicv3_its_collect_quirks() to gather
the effective workaround flags from both the IIDR-matched quirk entry
and the "dma-noncoherent" device-tree property.
This lets non-coherent platforms force non-cacheable ITS table
attributes even when no IIDR quirk entry matches.
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
xen/arch/arm/gic-v3-its.c | 70 ++++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 27 deletions(-)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 9ba068c46f..00524b43a3 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -57,71 +57,87 @@ struct its_device {
*/
struct its_quirk {
const char *desc;
- bool (*init)(struct host_its *hw_its);
uint32_t iidr;
uint32_t mask;
+ uint32_t 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,
+ .desc = "R-Car Gen4",
+ .iidr = 0x0201743b,
+ .mask = 0xffffffffU,
+ .flags = HOST_ITS_WORKAROUND_NC_NS |
+ HOST_ITS_WORKAROUND_32BIT_ADDR,
},
{
/* Sentinel. */
}
};
-static struct its_quirk* gicv3_its_find_quirk(uint32_t iidr)
+static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
{
- const struct its_quirk *quirks = its_quirks;
+ const struct its_quirk *quirk = its_quirks;
- for ( ; quirks->desc; quirks++ )
+ for ( ; quirk->desc; quirk++ )
{
- if ( quirks->iidr == (quirks->mask & iidr) )
- return (struct its_quirk *)quirks;
+ if ( quirk->iidr != (quirk->mask & iidr) )
+ continue;
+
+ return quirk;
}
return NULL;
}
-static void gicv3_its_enable_quirks(struct host_its *hw_its)
+static uint32_t gicv3_its_collect_quirks(const struct host_its *hw_its,
+ const struct its_quirk **matched_quirk)
{
+ const struct its_quirk *quirk;
+ uint32_t flags = 0;
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) )
+ quirk = gicv3_its_find_quirk(iidr);
+ if ( quirk )
+ flags |= quirk->flags;
+
+ if ( hw_its->dt_node &&
+ dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
+ flags |= HOST_ITS_WORKAROUND_NC_NS;
+
+ if ( matched_quirk )
+ *matched_quirk = quirk;
+
+ return flags;
+}
+
+static void gicv3_its_enable_quirks(struct host_its *hw_its)
+{
+ const struct its_quirk *quirk;
+
+ its_quirk_flags = gicv3_its_collect_quirks(hw_its, &quirk);
+
+ if ( quirk )
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;
+ uint32_t quirks, prev_quirks;
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));
+ prev_quirks = gicv3_its_collect_quirks(hw_its, NULL);
- list_for_each_entry(hw_its, &host_its_list, entry)
+ list_for_each_entry_continue(hw_its, &host_its_list, entry)
{
- quirk = gicv3_its_find_quirk(readl_relaxed(hw_its->its_base + GITS_IIDR));
- BUG_ON(quirk != prev);
- prev = quirk;
+ quirks = gicv3_its_collect_quirks(hw_its, NULL);
+ BUG_ON(quirks != prev_quirks);
}
}
--
2.43.0
Hi Mykola,
Mykola Kvach <xakep.amatop@gmail.com> writes:
> From: Mykola Kvach <mykola_kvach@epam.com>
>
> Replace the per-quirk init callback with declarative flags in
> struct its_quirk, and introduce gicv3_its_collect_quirks() to gather
> the effective workaround flags from both the IIDR-matched quirk entry
> and the "dma-noncoherent" device-tree property.
>
> This lets non-coherent platforms force non-cacheable ITS table
> attributes even when no IIDR quirk entry matches.
>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> xen/arch/arm/gic-v3-its.c | 70 ++++++++++++++++++++++++---------------
> 1 file changed, 43 insertions(+), 27 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 9ba068c46f..00524b43a3 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -57,71 +57,87 @@ struct its_device {
> */
> struct its_quirk {
> const char *desc;
> - bool (*init)(struct host_its *hw_its);
> uint32_t iidr;
> uint32_t mask;
> + uint32_t 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,
> + .desc = "R-Car Gen4",
> + .iidr = 0x0201743b,
> + .mask = 0xffffffffU,
> + .flags = HOST_ITS_WORKAROUND_NC_NS |
> + HOST_ITS_WORKAROUND_32BIT_ADDR,
> },
> {
> /* Sentinel. */
> }
> };
>
> -static struct its_quirk* gicv3_its_find_quirk(uint32_t iidr)
> +static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
> {
> - const struct its_quirk *quirks = its_quirks;
> + const struct its_quirk *quirk = its_quirks;
>
> - for ( ; quirks->desc; quirks++ )
> + for ( ; quirk->desc; quirk++ )
> {
> - if ( quirks->iidr == (quirks->mask & iidr) )
> - return (struct its_quirk *)quirks;
> + if ( quirk->iidr != (quirk->mask & iidr) )
> + continue;
> +
> + return quirk;
> }
>
> return NULL;
> }
>
> -static void gicv3_its_enable_quirks(struct host_its *hw_its)
> +static uint32_t gicv3_its_collect_quirks(const struct host_its *hw_its,
> + const struct its_quirk **matched_quirk)
> {
> + const struct its_quirk *quirk;
> + uint32_t flags = 0;
> 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) )
> + quirk = gicv3_its_find_quirk(iidr);
> + if ( quirk )
> + flags |= quirk->flags;
> +
> + if ( hw_its->dt_node &&
> + dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
> + flags |= HOST_ITS_WORKAROUND_NC_NS;
> +
> + if ( matched_quirk )
> + *matched_quirk = quirk;
> +
> + return flags;
> +}
> +
> +static void gicv3_its_enable_quirks(struct host_its *hw_its)
> +{
> + const struct its_quirk *quirk;
> +
> + its_quirk_flags = gicv3_its_collect_quirks(hw_its, &quirk);
> +
> + if ( quirk )
> 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;
> + uint32_t quirks, prev_quirks;
> 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));
> + prev_quirks = gicv3_its_collect_quirks(hw_its, NULL);
>
> - list_for_each_entry(hw_its, &host_its_list, entry)
> + list_for_each_entry_continue(hw_its, &host_its_list, entry)
> {
> - quirk = gicv3_its_find_quirk(readl_relaxed(hw_its->its_base + GITS_IIDR));
> - BUG_ON(quirk != prev);
> - prev = quirk;
> + quirks = gicv3_its_collect_quirks(hw_its, NULL);
> + BUG_ON(quirks != prev_quirks);
I know it was in the previous version, but as you are already touching
this... This is not Xen BUG(). This is a platform problem. So you need
to panic here. Something like
if (quirks != prev_quirks)
panic("Different ITS instances has different quirks")
Also, I want to point out that you are not validating "dma-noncoherent"
quirk here. I mean, some ITS entries can have this property, some other
- don't. This makes me think that you need to promote this
"dma-noncoherent" quirk from open coded check to a `struct
its_quirk` entry, so it will be handled in generic way.
--
WBR, Volodymyr
Hi Volodymyr,
Thank you for the review.
On Wed, Mar 25, 2026 at 4:42 PM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> Hi Mykola,
>
> Mykola Kvach <xakep.amatop@gmail.com> writes:
>
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > Replace the per-quirk init callback with declarative flags in
> > struct its_quirk, and introduce gicv3_its_collect_quirks() to gather
> > the effective workaround flags from both the IIDR-matched quirk entry
> > and the "dma-noncoherent" device-tree property.
> >
> > This lets non-coherent platforms force non-cacheable ITS table
> > attributes even when no IIDR quirk entry matches.
> >
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > xen/arch/arm/gic-v3-its.c | 70 ++++++++++++++++++++++++---------------
> > 1 file changed, 43 insertions(+), 27 deletions(-)
> >
> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> > index 9ba068c46f..00524b43a3 100644
> > --- a/xen/arch/arm/gic-v3-its.c
> > +++ b/xen/arch/arm/gic-v3-its.c
> > @@ -57,71 +57,87 @@ struct its_device {
> > */
> > struct its_quirk {
> > const char *desc;
> > - bool (*init)(struct host_its *hw_its);
> > uint32_t iidr;
> > uint32_t mask;
> > + uint32_t 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,
> > + .desc = "R-Car Gen4",
> > + .iidr = 0x0201743b,
> > + .mask = 0xffffffffU,
> > + .flags = HOST_ITS_WORKAROUND_NC_NS |
> > + HOST_ITS_WORKAROUND_32BIT_ADDR,
> > },
> > {
> > /* Sentinel. */
> > }
> > };
> >
> > -static struct its_quirk* gicv3_its_find_quirk(uint32_t iidr)
> > +static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
> > {
> > - const struct its_quirk *quirks = its_quirks;
> > + const struct its_quirk *quirk = its_quirks;
> >
> > - for ( ; quirks->desc; quirks++ )
> > + for ( ; quirk->desc; quirk++ )
> > {
> > - if ( quirks->iidr == (quirks->mask & iidr) )
> > - return (struct its_quirk *)quirks;
> > + if ( quirk->iidr != (quirk->mask & iidr) )
> > + continue;
> > +
> > + return quirk;
> > }
> >
> > return NULL;
> > }
> >
> > -static void gicv3_its_enable_quirks(struct host_its *hw_its)
> > +static uint32_t gicv3_its_collect_quirks(const struct host_its *hw_its,
> > + const struct its_quirk **matched_quirk)
> > {
> > + const struct its_quirk *quirk;
> > + uint32_t flags = 0;
> > 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) )
> > + quirk = gicv3_its_find_quirk(iidr);
> > + if ( quirk )
> > + flags |= quirk->flags;
> > +
> > + if ( hw_its->dt_node &&
> > + dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
> > + flags |= HOST_ITS_WORKAROUND_NC_NS;
> > +
> > + if ( matched_quirk )
> > + *matched_quirk = quirk;
> > +
> > + return flags;
> > +}
> > +
> > +static void gicv3_its_enable_quirks(struct host_its *hw_its)
> > +{
> > + const struct its_quirk *quirk;
> > +
> > + its_quirk_flags = gicv3_its_collect_quirks(hw_its, &quirk);
> > +
> > + if ( quirk )
> > 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;
> > + uint32_t quirks, prev_quirks;
> > 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));
> > + prev_quirks = gicv3_its_collect_quirks(hw_its, NULL);
> >
> > - list_for_each_entry(hw_its, &host_its_list, entry)
> > + list_for_each_entry_continue(hw_its, &host_its_list, entry)
> > {
> > - quirk = gicv3_its_find_quirk(readl_relaxed(hw_its->its_base + GITS_IIDR));
> > - BUG_ON(quirk != prev);
> > - prev = quirk;
> > + quirks = gicv3_its_collect_quirks(hw_its, NULL);
> > + BUG_ON(quirks != prev_quirks);
>
> I know it was in the previous version, but as you are already touching
> this... This is not Xen BUG(). This is a platform problem. So you need
> to panic here. Something like
>
> if (quirks != prev_quirks)
> panic("Different ITS instances has different quirks")
Ack.
>
>
> Also, I want to point out that you are not validating "dma-noncoherent"
> quirk here. I mean, some ITS entries can have this property, some other
> - don't. This makes me think that you need to promote this
> "dma-noncoherent" quirk from open coded check to a `struct
> its_quirk` entry, so it will be handled in generic way.
Just to clarify your point about dma-noncoherent:
In the current version it is already part of the effective quirk set,
because gicv3_its_validate_quirks() compares the flags returned by
gicv3_its_collect_quirks(), and those already include the
dma-noncoherent DT property.
So is your concern that DT-derived ITS properties such as
dma-noncoherent should also go through the same common quirk/collection
plumbing as the other ITS workaround sources, rather than being handled
as a separate open-coded check, similar to how Linux models this [1]?
Or did you have something else in mind?
Best regards,
Mykola
[1] https://elixir.bootlin.com/linux/v6.19.9/source/drivers/irqchip/irq-gic-v3-its.c#L4973
>
>
> --
> WBR, Volodymyr
Hi Mykola,
Mykola Kvach <xakep.amatop@gmail.com> writes:
> Hi Volodymyr,
>
> Thank you for the review.
>
> On Wed, Mar 25, 2026 at 4:42 PM Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com> wrote:
>>
>> Hi Mykola,
>>
>> Mykola Kvach <xakep.amatop@gmail.com> writes:
>>
>> > From: Mykola Kvach <mykola_kvach@epam.com>
>> >
>> > Replace the per-quirk init callback with declarative flags in
>> > struct its_quirk, and introduce gicv3_its_collect_quirks() to gather
>> > the effective workaround flags from both the IIDR-matched quirk entry
>> > and the "dma-noncoherent" device-tree property.
>> >
>> > This lets non-coherent platforms force non-cacheable ITS table
>> > attributes even when no IIDR quirk entry matches.
>> >
>> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>> > ---
>> > xen/arch/arm/gic-v3-its.c | 70 ++++++++++++++++++++++++---------------
>> > 1 file changed, 43 insertions(+), 27 deletions(-)
>> >
>> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> > index 9ba068c46f..00524b43a3 100644
>> > --- a/xen/arch/arm/gic-v3-its.c
>> > +++ b/xen/arch/arm/gic-v3-its.c
>> > @@ -57,71 +57,87 @@ struct its_device {
>> > */
>> > struct its_quirk {
>> > const char *desc;
>> > - bool (*init)(struct host_its *hw_its);
>> > uint32_t iidr;
>> > uint32_t mask;
>> > + uint32_t 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,
>> > + .desc = "R-Car Gen4",
>> > + .iidr = 0x0201743b,
>> > + .mask = 0xffffffffU,
>> > + .flags = HOST_ITS_WORKAROUND_NC_NS |
>> > + HOST_ITS_WORKAROUND_32BIT_ADDR,
>> > },
>> > {
>> > /* Sentinel. */
>> > }
>> > };
>> >
>> > -static struct its_quirk* gicv3_its_find_quirk(uint32_t iidr)
>> > +static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
>> > {
>> > - const struct its_quirk *quirks = its_quirks;
>> > + const struct its_quirk *quirk = its_quirks;
>> >
>> > - for ( ; quirks->desc; quirks++ )
>> > + for ( ; quirk->desc; quirk++ )
>> > {
>> > - if ( quirks->iidr == (quirks->mask & iidr) )
>> > - return (struct its_quirk *)quirks;
>> > + if ( quirk->iidr != (quirk->mask & iidr) )
>> > + continue;
>> > +
>> > + return quirk;
>> > }
>> >
>> > return NULL;
>> > }
>> >
>> > -static void gicv3_its_enable_quirks(struct host_its *hw_its)
>> > +static uint32_t gicv3_its_collect_quirks(const struct host_its *hw_its,
>> > + const struct its_quirk **matched_quirk)
>> > {
>> > + const struct its_quirk *quirk;
>> > + uint32_t flags = 0;
>> > 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) )
>> > + quirk = gicv3_its_find_quirk(iidr);
>> > + if ( quirk )
>> > + flags |= quirk->flags;
>> > +
>> > + if ( hw_its->dt_node &&
>> > + dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
>> > + flags |= HOST_ITS_WORKAROUND_NC_NS;
>> > +
>> > + if ( matched_quirk )
>> > + *matched_quirk = quirk;
>> > +
>> > + return flags;
>> > +}
>> > +
>> > +static void gicv3_its_enable_quirks(struct host_its *hw_its)
>> > +{
>> > + const struct its_quirk *quirk;
>> > +
>> > + its_quirk_flags = gicv3_its_collect_quirks(hw_its, &quirk);
>> > +
>> > + if ( quirk )
>> > 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;
>> > + uint32_t quirks, prev_quirks;
>> > 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));
>> > + prev_quirks = gicv3_its_collect_quirks(hw_its, NULL);
>> >
>> > - list_for_each_entry(hw_its, &host_its_list, entry)
>> > + list_for_each_entry_continue(hw_its, &host_its_list, entry)
>> > {
>> > - quirk = gicv3_its_find_quirk(readl_relaxed(hw_its->its_base + GITS_IIDR));
>> > - BUG_ON(quirk != prev);
>> > - prev = quirk;
>> > + quirks = gicv3_its_collect_quirks(hw_its, NULL);
>> > + BUG_ON(quirks != prev_quirks);
>>
>> I know it was in the previous version, but as you are already touching
>> this... This is not Xen BUG(). This is a platform problem. So you need
>> to panic here. Something like
>>
>> if (quirks != prev_quirks)
>> panic("Different ITS instances has different quirks")
>
> Ack.
>
>
>>
>>
>> Also, I want to point out that you are not validating "dma-noncoherent"
>> quirk here. I mean, some ITS entries can have this property, some other
>> - don't. This makes me think that you need to promote this
>> "dma-noncoherent" quirk from open coded check to a `struct
>> its_quirk` entry, so it will be handled in generic way.
>
> Just to clarify your point about dma-noncoherent:
>
> In the current version it is already part of the effective quirk set,
> because gicv3_its_validate_quirks() compares the flags returned by
> gicv3_its_collect_quirks(), and those already include the
> dma-noncoherent DT property.
>
> So is your concern that DT-derived ITS properties such as
> dma-noncoherent should also go through the same common quirk/collection
> plumbing as the other ITS workaround sources, rather than being handled
> as a separate open-coded check, similar to how Linux models this [1]?
Yes, exactly this. I see no point in introducing generic predicate
function and then not using it. This will confuse anyone who tries to
read the code.
--
WBR, Volodymyr
© 2016 - 2026 Red Hat, Inc.