Re: [PATCH 1/2] hw/intc: GICv3 ITS implementation

[Peter Maydell]

On Mon, 15 Mar 2021 at 16:49, Shashi Mallela <shashi.mallela@linaro.org> wrote:
>
> Implementation of Interrupt Translation Service which allows eventid
> from devices to be translated to physical LPI IntIds.Extended the
> redistributor functionality to process LPI Interrupts as well.
>
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  hw/intc/arm_gicv3.c                    |    6 +
>  hw/intc/arm_gicv3_common.c             |   20 +-
>  hw/intc/arm_gicv3_cpuif.c              |   15 +-
>  hw/intc/arm_gicv3_dist.c               |   22 +-
>  hw/intc/arm_gicv3_its.c                | 1386 ++++++++++++++++++++++++
>  hw/intc/arm_gicv3_its_common.c         |   17 +-
>  hw/intc/arm_gicv3_its_kvm.c            |    2 +-
>  hw/intc/arm_gicv3_redist.c             |  163 ++-
>  hw/intc/gicv3_internal.h               |  169 ++-
>  hw/intc/meson.build                    |    1 +
>  include/hw/intc/arm_gicv3_common.h     |   13 +
>  include/hw/intc/arm_gicv3_its_common.h |   12 +-
>  12 files changed, 1807 insertions(+), 19 deletions(-)
>  create mode 100644 hw/intc/arm_gicv3_its.c

Hi; thanks for posting this. Unfortuantely 1800 lines is much
too large a patch to be reviewable. Could you split the series
up into smaller pieces, please? One possible structure would be
to have a patch with the framework of the device but no actual
implementation of register behaviour or command handling,
followed by patches which add the behaviour piece by piece,
and then finally the patch adding it to the board.

I think it would also be useful to have the virt board
support, as a demonstration that the emulated ITS and
the KVM ITS have the same interface to the board code
and are basically drop-in-replacements.

Finally, for multi-patch series, please always send a cover letter
(the "0/5" email, which the other patch emails are followups to;
git format-patch should do this for you).

thanks
-- PMM