Re: [PATCH v2 3/4] xen/arm: its: refactor ITS quirk matching

[Mykola Kvach <mykola_kvach@xxxxxxxx>]

On Fri, Jul 31, 2026 at 10:34:11AM +0200, Orzel, Michal wrote:
> 
> 
> On 28-May-26 02:25, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> > 
> > ITS quirks are currently matched only by IIDR and mask fields stored in
> > each table entry. That is too coarse for integrations where the same GIC
> > IP block can appear in several platforms but the workaround is only valid
> > for a subset of boards.
> > 
> > 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. The R-Car Gen4 platform refinement is DT-only;
> > ACPI-discovered ITSes do not match it.
> > 
> > Keep first-match semantics explicit. Assert that non-sentinel entries
> > provide a matcher and that IIDR matching receives match data, but keep
> > runtime guards so a malformed table entry does not become a NULL function
> > call or NULL data dereference in non-debug builds. 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@xxxxxxxx>
> > ---
> > 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 | 67 +++++++++++++++++++++++++++++++--------
> >  1 file changed, 53 insertions(+), 14 deletions(-)
> > 
> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> > index dc48a84789..e055914763 100644
> > --- a/xen/arch/arm/gic-v3-its.c
> > +++ b/xen/arch/arm/gic-v3-its.c
> > @@ -53,8 +53,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
> > @@ -64,11 +64,48 @@ struct its_quirk {
> >      uint32_t lpi_flags;
> >  };
> >  
> > +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);
> > +
> > +    match = data;
> > +    iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
> > +
> > +    return (iidr & match->mask) == match->iidr;
> The commit message says you keep a runtime guard so malformed table data
> does not become a NULL data dereference in non-debug builds, but there is
> no such guard - ASSERT() compiles out and match->mask is then read from
> NULL.

You're right: ASSERT() alone does not provide the guard described in
the commit message. I'll add an explicit NULL check before the IIDR
match data is dereferenced, so it also works in non-debug builds.

> 
> > +}
> > +
> > +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") )
> Given that IIDR 0x0201743b is not Renesas-specific as you mention in the cover
> letter, this is a behavior changed and should be mentioned in the commit msg
> (cover letter is not in git).

I'll document the change from IIDR-only matching to platform-restricted
matching in the commit message, including its effect on ACPI and other
platforms.

> 
> > +        return false;
> > +
> > +    return gicv3_its_match_iidr(hw_its, data);
> > +}
> > +
> > +static const struct its_quirk_match_iidr rcar_gen4_iidr = {
> __initconst
> 
> > +    .iidr = 0x0201743b,
> > +    .mask = 0xffffffffU,
> > +};
> > +
> >  static const struct its_quirk its_quirks[] = {
> __initconstrel

I'll also mark the IIDR match data __initconst and the pointer-containing
quirk table __initconstrel.

Best regards,
Mykola