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

[Peter Maydell]

On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <shashi.mallela@linaro.org> 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>
>

> diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
> index f7675a5adc..43ce4a8a95 100644
> --- a/hw/intc/gicv3_internal.h
> +++ b/hw/intc/gicv3_internal.h
> @@ -68,6 +68,9 @@
>  #define GICD_CTLR_E1NWF             (1U << 7)
>  #define GICD_CTLR_RWP               (1U << 31)
>
> +#define GICD_TYPER_LPIS_OFFSET         17

This should be named GICD_TYPER_LPIS_SHIFT, in the usual naming convention.

> +#define GICD_TYPER_IDBITS_OFFSET       19
> +#define GICD_TYPER_IDBITS_MASK       0x1f

You never use these, so don't define them.

>  /* 16 bits EventId */
>  #define GICD_TYPER_IDBITS            0xf
>
> @@ -123,6 +126,9 @@
>  #define GICR_TYPER_COMMONLPIAFF      (0x3 << 24)
>  #define GICR_TYPER_AFFINITYVALUE     (0xFFFFFFFFULL << 32)
>
> +#define GICR_WAKER_ProcessorSleep    (1U << 1)
> +#define GICR_WAKER_ChildrenAsleep    (1U << 2)
> +
>  FIELD(GICR_PROPBASER, IDBITS, 0, 5)
>  FIELD(GICR_PROPBASER, INNERCACHE, 7, 3)
>  FIELD(GICR_PROPBASER, SHAREABILITY, 10, 2)
> @@ -137,9 +143,6 @@ FIELD(GICR_PENDBASER, PHYADDR, 16, 36)
>  FIELD(GICR_PENDBASER, OUTERCACHE, 56, 3)
>  FIELD(GICR_PENDBASER, PTZ, 62, 1)
>
> -#define GICR_WAKER_ProcessorSleep    (1U << 1)
> -#define GICR_WAKER_ChildrenAsleep    (1U << 2)
> -

Why move these defines?

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM