Re: [PATCH v7 05/10] hw/intc: GICv3 ITS Feature enablement

[Neil Armstrong]

On 06/08/2021 00:29, Shashi Mallela wrote:
> Added properties to enable ITS feature and define qemu system
> address space memory in gicv3 common,setup distributor and
> redistributor registers to indicate LPI support.
> 
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/intc/arm_gicv3_common.c         | 12 ++++++++++++
>  hw/intc/arm_gicv3_dist.c           |  5 ++++-
>  hw/intc/arm_gicv3_redist.c         | 12 +++++++++---
>  hw/intc/gicv3_internal.h           |  2 ++
>  include/hw/intc/arm_gicv3_common.h |  1 +
>  5 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 58ef65f589..53dea2a775 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -345,6 +345,11 @@ static void arm_gicv3_common_realize(DeviceState *dev, 
> Error **errp)
>          return;
>      }
>  
> +    if (s->lpi_enable && !s->dma) {
> +        error_setg(errp, "Redist-ITS: Guest 'sysmem' reference link not 
> set");
> +        return;
> +    }
> +
>      s->cpu = g_new0(GICv3CPUState, s->num_cpu);
>  
>      for (i = 0; i < s->num_cpu; i++) {
> @@ -381,6 +386,10 @@ static void arm_gicv3_common_realize(DeviceState *dev, 
> Error **errp)
>              (1 << 24) |
>              (i << 8) |
>              (last << 4);
> +
> +        if (s->lpi_enable) {
> +            s->cpu[i].gicr_typer |= GICR_TYPER_PLPIS;
> +        }
>      }
>  }
>  
> @@ -494,9 +503,12 @@ static Property arm_gicv3_common_properties[] = {
>      DEFINE_PROP_UINT32("num-cpu", GICv3State, num_cpu, 1),
>      DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
>      DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
> +    DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
>      DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 
> 0),
>      DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions,
>                        redist_region_count, qdev_prop_uint32, uint32_t),
> +    DEFINE_PROP_LINK("sysmem", GICv3State, dma, TYPE_MEMORY_REGION,
> +                     MemoryRegion *),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
> index b65f56f903..43128b376d 100644
> --- a/hw/intc/arm_gicv3_dist.c
> +++ b/hw/intc/arm_gicv3_dist.c
> @@ -371,7 +371,9 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr 
> offset,
>           * A3V == 1 (non-zero values of Affinity level 3 supported)
>           * IDbits == 0xf (we support 16-bit interrupt identifiers)
>           * DVIS == 0 (Direct virtual LPI injection not supported)
> -         * LPIS == 0 (LPIs not supported)
> +         * LPIS == 1 (LPIs are supported if affinity routing is enabled)
> +         * num_LPIs == 0b00000 (bits [15:11],Number of LPIs as indicated
> +         *                      by GICD_TYPER.IDbits)
>           * MBIS == 0 (message-based SPIs not supported)
>           * SecurityExtn == 1 if security extns supported
>           * CPUNumber == 0 since for us ARE is always 1
> @@ -386,6 +388,7 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr 
> offset,
>          bool sec_extn = !(s->gicd_ctlr & GICD_CTLR_DS);
>  
>          *data = (1 << 25) | (1 << 24) | (sec_extn << 10) |
> +            (s->lpi_enable << GICD_TYPER_LPIS_SHIFT) |
>              (0xf << 19) | itlinesnumber;
>          return MEMTX_OK;
>      }
> diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> index 53da703ed8..2108abfe9c 100644
> --- a/hw/intc/arm_gicv3_redist.c
> +++ b/hw/intc/arm_gicv3_redist.c
> @@ -248,10 +248,16 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, 
> hwaddr offset,
>      case GICR_CTLR:
>          /* For our implementation, GICR_TYPER.DPGS is 0 and so all
>           * the DPG bits are RAZ/WI. We don't do anything asynchronously,
> -         * so UWP and RWP are RAZ/WI. And GICR_TYPER.LPIS is 0 (we don't
> -         * implement LPIs) so Enable_LPIs is RES0. So there are no writable
> -         * bits for us.
> +         * so UWP and RWP are RAZ/WI. GICR_TYPER.LPIS is 1 (we
> +         * implement LPIs) so Enable_LPIs is programmable.
>           */
> +        if (cs->gicr_typer & GICR_TYPER_PLPIS) {
> +            if (value & GICR_CTLR_ENABLE_LPIS) {
> +                cs->gicr_ctlr |= GICR_CTLR_ENABLE_LPIS;
> +            } else {
> +                cs->gicr_ctlr &= ~GICR_CTLR_ENABLE_LPIS;
> +            }
> +        }
>          return MEMTX_OK;
>      case GICR_STATUSR:
>          /* RAZ/WI for our implementation */
> diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
> index 1966444790..530d1c1789 100644
> --- a/hw/intc/gicv3_internal.h
> +++ b/hw/intc/gicv3_internal.h
> @@ -68,6 +68,8 @@
>  #define GICD_CTLR_E1NWF             (1U << 7)
>  #define GICD_CTLR_RWP               (1U << 31)
>  
> +#define GICD_TYPER_LPIS_SHIFT          17
> +
>  /* 16 bits EventId */
>  #define GICD_TYPER_IDBITS            0xf
>  
> diff --git a/include/hw/intc/arm_gicv3_common.h 
> b/include/hw/intc/arm_gicv3_common.h
> index 0715b0bc2a..c1348cc60a 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -221,6 +221,7 @@ struct GICv3State {
>      uint32_t num_cpu;
>      uint32_t num_irq;
>      uint32_t revision;
> +    bool lpi_enable;
>      bool security_extn;
>      bool irq_reset_nonsecure;
>      bool gicd_no_migration_shift_bug;
> 


Tested with in-review Zephyr ITS implementation at 
https://github.com/zephyrproject-rtos/zephyr/pull/37506

Tested-by: Neil Armstrong <narmstrong@baylibre.com>