Re: [PATCH 2/4] xen/arm: its: add platform match callback for ITS quirks

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi Mykola,

Mykola Kvach <xakep.amatop@xxxxxxxxx> writes:

> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
>
> Extend ITS quirk lookup with an optional match callback so that
> platforms sharing the same IIDR can still be distinguished.
>
> Use the board compatible string to positively identify Renesas R-Car
> Gen4 before applying ITS workaround flags, preventing false matches
> on other SoCs that happen to use the same GIC IP block.
>
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
>  xen/arch/arm/gic-v3-its.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 00524b43a3..c40629731f 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -57,6 +57,7 @@ struct its_device {
>   */
>  struct its_quirk {
>      const char *desc;
> +    bool (*match)(const struct host_its *hw_its);

If you are introducing match predicate, then why do you need...

>      uint32_t iidr;
>      uint32_t mask;
>      uint32_t flags;

these? You can use a predicate function to match against iidr

> @@ -64,11 +65,24 @@ struct its_quirk {
>  
>  static uint32_t __ro_after_init its_quirk_flags;
>  
> +static bool gicv3_its_match_quirk_gen4(const struct host_its *hw_its)
> +{
> +    if ( !hw_its->dt_node )
> +        return false;
> +
> +    if ( !dt_machine_is_compatible("renesas,r8a779f0") &&
> +         !dt_machine_is_compatible("renesas,r8a779g0") )
> +        return false;
> +
> +    return true;
> +}
> +
>  static const struct its_quirk its_quirks[] = {
>      {
>          .desc  = "R-Car Gen4",
>          .iidr  = 0x0201743b,
>          .mask  = 0xffffffffU,
> +        .match = gicv3_its_match_quirk_gen4,
>          .flags = HOST_ITS_WORKAROUND_NC_NS |
>                   HOST_ITS_WORKAROUND_32BIT_ADDR,
>      },
> @@ -77,7 +91,8 @@ static const struct its_quirk its_quirks[] = {
>      }
>  };
>  
> -static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
> +static const struct its_quirk *gicv3_its_find_quirk(
> +    const struct host_its *hw_its, uint32_t iidr)
>  {
>      const struct its_quirk *quirk = its_quirks;
>  
> @@ -86,7 +101,8 @@ static const struct its_quirk 
> *gicv3_its_find_quirk(uint32_t iidr)
>          if ( quirk->iidr != (quirk->mask & iidr) )
>              continue;
>  
> -        return quirk;
> +        if ( !quirk->match || quirk->match(hw_its) )
> +            return quirk;
>      }
>  
>      return NULL;
> @@ -99,7 +115,7 @@ static uint32_t gicv3_its_collect_quirks(const struct 
> host_its *hw_its,
>      uint32_t flags = 0;
>      uint32_t iidr = readl_relaxed(hw_its->its_base + GITS_IIDR);
>  
> -    quirk = gicv3_its_find_quirk(iidr);
> +    quirk = gicv3_its_find_quirk(hw_its, iidr);
>      if ( quirk )
>          flags |= quirk->flags;

-- 
WBR, Volodymyr