diff mbox series

[v5,01/10] hw/intc: GICv3 ITS initial framework

Message ID 20210630153156.9421-2-shashi.mallela@linaro.org (mailing list archive)
State New, archived
Headers show
Series GICv3 LPI and ITS feature implementation | expand

Commit Message

Shashi Mallela June 30, 2021, 3:31 p.m. UTC
Added register definitions relevant to ITS,implemented overall
ITS device framework with stubs for ITS control and translater
regions read/write,extended ITS common to handle mmio init between
existing kvm device and newer qemu device.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gicv3_its.c                | 240 +++++++++++++++++++++++++
 hw/intc/arm_gicv3_its_common.c         |   7 +-
 hw/intc/arm_gicv3_its_kvm.c            |   2 +-
 hw/intc/gicv3_internal.h               |  88 +++++++--
 hw/intc/meson.build                    |   1 +
 include/hw/intc/arm_gicv3_its_common.h |   9 +-
 6 files changed, 331 insertions(+), 16 deletions(-)
 create mode 100644 hw/intc/arm_gicv3_its.c

Comments

Peter Maydell July 5, 2021, 2:58 p.m. UTC | #1
On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <shashi.mallela@linaro.org> wrote:
>
> Added register definitions relevant to ITS,implemented overall
> ITS device framework with stubs for ITS control and translater
> regions read/write,extended ITS common to handle mmio init between
> existing kvm device and newer qemu device.
>
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
> +{
> +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> +
> +    gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_translation_ops);
> +
> +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {

Can you remind me why we make this check, please? When would we
have created an ITS device but not have a GICv3 with LPI support?

Maybe it would be better to either
(a) simply create the ITS and assume that the board connected it up
to a GICv3 that supports it
(b) check every CPU for whether PLPIS is set, and if one of them does
not have it set then return an error from the ITS realize

?

(Found this by looking for code where we do s->gicv3->cpu->something...)

> +        /* set the ITS default features supported */
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL,
> +                              GITS_TYPE_PHYSICAL);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE,
> +                              ITS_ITT_ENTRY_SIZE - 1);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, ITS_IDBITS);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, DEVBITS, ITS_DEVBITS);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIL, 1);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIDBITS, ITS_CIDBITS);
> +    }
> +}
> +
> +static void gicv3_its_reset(DeviceState *dev)
> +{
> +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> +
> +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {

Similarly here.

> +        c->parent_reset(dev);
> +
> +        /* Quiescent bit reset to 1 */
> +        s->ctlr = FIELD_DP32(s->ctlr, GITS_CTLR, QUIESCENT, 1);
> +
> +        /*
> +         * setting GITS_BASER0.Type = 0b001 (Device)
> +         *         GITS_BASER1.Type = 0b100 (Collection Table)
> +         *         GITS_BASER<n>.Type,where n = 3 to 7 are 0b00 (Unimplemented)
> +         *         GITS_BASER<0,1>.Page_Size = 64KB
> +         * and default translation table entry size to 16 bytes
> +         */
> +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, TYPE,
> +                                 GITS_ITT_TYPE_DEVICE);
> +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, PAGESIZE,
> +                                 GITS_BASER_PAGESIZE_64K);
> +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, ENTRYSIZE,
> +                                 GITS_DTE_SIZE - 1);
> +
> +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, TYPE,
> +                                 GITS_ITT_TYPE_COLLECTION);
> +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, PAGESIZE,
> +                                 GITS_BASER_PAGESIZE_64K);
> +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, ENTRYSIZE,
> +                                 GITS_CTE_SIZE - 1);
> +    }
> +}

thanks
-- PMM
Shashi Mallela July 5, 2021, 3:55 p.m. UTC | #2
On Mon, 2021-07-05 at 15:58 +0100, Peter Maydell wrote:
> On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <
> shashi.mallela@linaro.org> wrote:
> > Added register definitions relevant to ITS,implemented overall
> > ITS device framework with stubs for ITS control and translater
> > regions read/write,extended ITS common to handle mmio init between
> > existing kvm device and newer qemu device.
> > 
> > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
> > +{
> > +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> > +
> > +    gicv3_its_init_mmio(s, &gicv3_its_control_ops,
> > &gicv3_its_translation_ops);
> > +
> > +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> 
> Can you remind me why we make this check, please? When would we
> have created an ITS device but not have a GICv3 with LPI support?
This check applies to GIC's physical LPI support only as against GIC's
virtual LPI support. 
> 
> Maybe it would be better to either
> (a) simply create the ITS and assume that the board connected it up
> to a GICv3 that supports it
> (b) check every CPU for whether PLPIS is set, and if one of them does
> not have it set then return an error from the ITS realize
> 
> ?
> 
> (Found this by looking for code where we do s->gicv3->cpu-
> >something...)
> 
> > +        /* set the ITS default features supported */
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL,
> > +                              GITS_TYPE_PHYSICAL);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER,
> > ITT_ENTRY_SIZE,
> > +                              ITS_ITT_ENTRY_SIZE - 1);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS,
> > ITS_IDBITS);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, DEVBITS,
> > ITS_DEVBITS);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIL, 1);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIDBITS,
> > ITS_CIDBITS);
> > +    }
> > +}
> > +
> > +static void gicv3_its_reset(DeviceState *dev)
> > +{
> > +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> > +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> > +
> > +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> 
> Similarly here.
> 
> > +        c->parent_reset(dev);
> > +
> > +        /* Quiescent bit reset to 1 */
> > +        s->ctlr = FIELD_DP32(s->ctlr, GITS_CTLR, QUIESCENT, 1);
> > +
> > +        /*
> > +         * setting GITS_BASER0.Type = 0b001 (Device)
> > +         *         GITS_BASER1.Type = 0b100 (Collection Table)
> > +         *         GITS_BASER<n>.Type,where n = 3 to 7 are 0b00
> > (Unimplemented)
> > +         *         GITS_BASER<0,1>.Page_Size = 64KB
> > +         * and default translation table entry size to 16 bytes
> > +         */
> > +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, TYPE,
> > +                                 GITS_ITT_TYPE_DEVICE);
> > +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER,
> > PAGESIZE,
> > +                                 GITS_BASER_PAGESIZE_64K);
> > +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER,
> > ENTRYSIZE,
> > +                                 GITS_DTE_SIZE - 1);
> > +
> > +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, TYPE,
> > +                                 GITS_ITT_TYPE_COLLECTION);
> > +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER,
> > PAGESIZE,
> > +                                 GITS_BASER_PAGESIZE_64K);
> > +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER,
> > ENTRYSIZE,
> > +                                 GITS_CTE_SIZE - 1);
> > +    }
> > +}
> 
> thanks
> -- PMM
Peter Maydell July 5, 2021, 4:25 p.m. UTC | #3
On Mon, 5 Jul 2021 at 16:55, <shashi.mallela@linaro.org> wrote:
>
> On Mon, 2021-07-05 at 15:58 +0100, Peter Maydell wrote:
> > On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <
> > shashi.mallela@linaro.org> wrote:
> > > Added register definitions relevant to ITS,implemented overall
> > > ITS device framework with stubs for ITS control and translater
> > > regions read/write,extended ITS common to handle mmio init between
> > > existing kvm device and newer qemu device.
> > >
> > > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > > +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
> > > +{
> > > +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> > > +
> > > +    gicv3_its_init_mmio(s, &gicv3_its_control_ops,
> > > &gicv3_its_translation_ops);
> > > +
> > > +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> >
> > Can you remind me why we make this check, please? When would we
> > have created an ITS device but not have a GICv3 with LPI support?
> This check applies to GIC's physical LPI support only as against GIC's
> virtual LPI support.

Right, but when would we have a GIC with no physical LPI support
but an ITS is present ?

thanks
-- PMM
Shashi Mallela July 5, 2021, 5:04 p.m. UTC | #4
On Mon, 2021-07-05 at 17:25 +0100, Peter Maydell wrote:
> On Mon, 5 Jul 2021 at 16:55, <shashi.mallela@linaro.org> wrote:
> > On Mon, 2021-07-05 at 15:58 +0100, Peter Maydell wrote:
> > > On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <
> > > shashi.mallela@linaro.org> wrote:
> > > > Added register definitions relevant to ITS,implemented overall
> > > > ITS device framework with stubs for ITS control and translater
> > > > regions read/write,extended ITS common to handle mmio init
> > > > between
> > > > existing kvm device and newer qemu device.
> > > > 
> > > > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> > > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > > > +static void gicv3_arm_its_realize(DeviceState *dev, Error
> > > > **errp)
> > > > +{
> > > > +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> > > > +
> > > > +    gicv3_its_init_mmio(s, &gicv3_its_control_ops,
> > > > &gicv3_its_translation_ops);
> > > > +
> > > > +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> > > 
> > > Can you remind me why we make this check, please? When would we
> > > have created an ITS device but not have a GICv3 with LPI support?
> > This check applies to GIC's physical LPI support only as against
> > GIC's
> > virtual LPI support.
> 
> Right, but when would we have a GIC with no physical LPI support
> but an ITS is present ?
If we only support Direct injection of virtual interrupts (which can
have their own vPEID and the vPE table),then the ITS present could havejust virtual LPI support 
> 
> thanks
> -- PMM
Peter Maydell July 5, 2021, 6:58 p.m. UTC | #5
On Mon, 5 Jul 2021 at 18:04, <shashi.mallela@linaro.org> wrote:
>
> On Mon, 2021-07-05 at 17:25 +0100, Peter Maydell wrote:
> > On Mon, 5 Jul 2021 at 16:55, <shashi.mallela@linaro.org> wrote:
> > > On Mon, 2021-07-05 at 15:58 +0100, Peter Maydell wrote:
> > > > On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <
> > > > shashi.mallela@linaro.org> wrote:
> > > > > Added register definitions relevant to ITS,implemented overall
> > > > > ITS device framework with stubs for ITS control and translater
> > > > > regions read/write,extended ITS common to handle mmio init
> > > > > between
> > > > > existing kvm device and newer qemu device.
> > > > >
> > > > > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> > > > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > > > > +static void gicv3_arm_its_realize(DeviceState *dev, Error
> > > > > **errp)
> > > > > +{
> > > > > +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> > > > > +
> > > > > +    gicv3_its_init_mmio(s, &gicv3_its_control_ops,
> > > > > &gicv3_its_translation_ops);
> > > > > +
> > > > > +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> > > >
> > > > Can you remind me why we make this check, please? When would we
> > > > have created an ITS device but not have a GICv3 with LPI support?
> > > This check applies to GIC's physical LPI support only as against
> > > GIC's
> > > virtual LPI support.
> >
> > Right, but when would we have a GIC with no physical LPI support
> > but an ITS is present ?
> If we only support Direct injection of virtual interrupts (which can
> have their own vPEID and the vPE table),then the ITS present could havejust virtual LPI support

This patchset does not support a virtual-LPI-only ITS, though:
it doesn't support virtual LPIs at all.
If you use it with CPUs without physical LPI support , this code will skip
entirely setting GITS_TYPER and will make reset do nothing, and then the
rest of the ITS implementation will misbehave.

I think what we should do is:
 * in realize, check every CPU to make sure its redistributor
   supports physical LPIs, and return an error from realize if not
 * in reset, don't check anything

If we add virtual-LPI-only ITS support later, we can always update
this code appropriately.

thanks
-- PMM
Eric Auger July 6, 2021, 7:44 a.m. UTC | #6
Hi,

On 6/30/21 5:31 PM, Shashi Mallela wrote:
> Added register definitions relevant to ITS,implemented overall
> ITS device framework with stubs for ITS control and translater
> regions read/write,extended ITS common to handle mmio init between
> existing kvm device and newer qemu device.
> 
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Some of my comments in v4 were not commented nor addressed in v5.

Also here and in the other respinned patches, please add an individual
history log to track the major changes you made from n-1 to n to help
the review.

Thanks

Eric
> ---
>  hw/intc/arm_gicv3_its.c                | 240 +++++++++++++++++++++++++
>  hw/intc/arm_gicv3_its_common.c         |   7 +-
>  hw/intc/arm_gicv3_its_kvm.c            |   2 +-
>  hw/intc/gicv3_internal.h               |  88 +++++++--
>  hw/intc/meson.build                    |   1 +
>  include/hw/intc/arm_gicv3_its_common.h |   9 +-
>  6 files changed, 331 insertions(+), 16 deletions(-)
>  create mode 100644 hw/intc/arm_gicv3_its.c
> 
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> new file mode 100644
> index 0000000000..545cda3665
> --- /dev/null
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -0,0 +1,240 @@
> +/*
> + * ITS emulation for a GICv3-based system
> + *
> + * Copyright Linaro.org 2021
> + *
> + * Authors:
> + *  Shashi Mallela <shashi.mallela@linaro.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
> + * option) any later version.  See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/intc/arm_gicv3_its_common.h"
> +#include "gicv3_internal.h"
> +#include "qom/object.h"
> +
> +typedef struct GICv3ITSClass GICv3ITSClass;
> +/* This is reusing the GICv3ITSState typedef from ARM_GICV3_ITS_COMMON */
> +DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
> +                     ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
> +
> +struct GICv3ITSClass {
> +    GICv3ITSCommonClass parent_class;
> +    void (*parent_reset)(DeviceState *dev);
> +};
> +
> +static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset,
> +                                               uint64_t data, unsigned size,
> +                                               MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
> +                              uint64_t value, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset,
> +                             uint64_t *data, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t value, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset,
> +                              uint64_t *data, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult gicv3_its_read(void *opaque, hwaddr offset, uint64_t *data,
> +                                  unsigned size, MemTxAttrs attrs)
> +{
> +    GICv3ITSState *s = (GICv3ITSState *)opaque;
> +    MemTxResult result;
> +
> +    switch (size) {
> +    case 4:
> +        result = its_readl(s, offset, data, attrs);
> +        break;
> +    case 8:
> +        result = its_readll(s, offset, data, attrs);
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
> +
> +    if (result == MEMTX_ERROR) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid guest read at offset " TARGET_FMT_plx
> +                      "size %u\n", __func__, offset, size);
> +        /*
> +         * The spec requires that reserved registers are RAZ/WI;
> +         * so use MEMTX_ERROR returns from leaf functions as a way to
> +         * trigger the guest-error logging but don't return it to
> +         * the caller, or we'll cause a spurious guest data abort.
> +         */
> +        result = MEMTX_OK;
> +        *data = 0;
> +    }
> +    return result;
> +}
> +
> +static MemTxResult gicv3_its_write(void *opaque, hwaddr offset, uint64_t data,
> +                                   unsigned size, MemTxAttrs attrs)
> +{
> +    GICv3ITSState *s = (GICv3ITSState *)opaque;
> +    MemTxResult result;
> +
> +    switch (size) {
> +    case 4:
> +        result = its_writel(s, offset, data, attrs);
> +        break;
> +    case 8:
> +        result = its_writell(s, offset, data, attrs);
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
> +
> +    if (result == MEMTX_ERROR) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid guest write at offset " TARGET_FMT_plx
> +                      "size %u\n", __func__, offset, size);
> +        /*
> +         * The spec requires that reserved registers are RAZ/WI;
> +         * so use MEMTX_ERROR returns from leaf functions as a way to
> +         * trigger the guest-error logging but don't return it to
> +         * the caller, or we'll cause a spurious guest data abort.
> +         */
> +        result = MEMTX_OK;
> +    }
> +    return result;
> +}
> +
> +static const MemoryRegionOps gicv3_its_control_ops = {
> +    .read_with_attrs = gicv3_its_read,
> +    .write_with_attrs = gicv3_its_write,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 4,
> +    .impl.max_access_size = 8,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const MemoryRegionOps gicv3_its_translation_ops = {
> +    .write_with_attrs = gicv3_its_translation_write,
> +    .valid.min_access_size = 2,
> +    .valid.max_access_size = 4,
> +    .impl.min_access_size = 2,
> +    .impl.max_access_size = 4,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
> +{
> +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> +
> +    gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_translation_ops);
> +
> +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> +        /* set the ITS default features supported */
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL,
> +                              GITS_TYPE_PHYSICAL);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE,
> +                              ITS_ITT_ENTRY_SIZE - 1);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, ITS_IDBITS);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, DEVBITS, ITS_DEVBITS);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIL, 1);
> +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIDBITS, ITS_CIDBITS);
> +    }
> +}
> +
> +static void gicv3_its_reset(DeviceState *dev)
> +{
> +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> +
> +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> +        c->parent_reset(dev);
> +
> +        /* Quiescent bit reset to 1 */
> +        s->ctlr = FIELD_DP32(s->ctlr, GITS_CTLR, QUIESCENT, 1);
> +
> +        /*
> +         * setting GITS_BASER0.Type = 0b001 (Device)
> +         *         GITS_BASER1.Type = 0b100 (Collection Table)
> +         *         GITS_BASER<n>.Type,where n = 3 to 7 are 0b00 (Unimplemented)
> +         *         GITS_BASER<0,1>.Page_Size = 64KB
> +         * and default translation table entry size to 16 bytes
> +         */
> +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, TYPE,
> +                                 GITS_ITT_TYPE_DEVICE);
> +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, PAGESIZE,
> +                                 GITS_BASER_PAGESIZE_64K);
> +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, ENTRYSIZE,
> +                                 GITS_DTE_SIZE - 1);
> +
> +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, TYPE,
> +                                 GITS_ITT_TYPE_COLLECTION);
> +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, PAGESIZE,
> +                                 GITS_BASER_PAGESIZE_64K);
> +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, ENTRYSIZE,
> +                                 GITS_CTE_SIZE - 1);
> +    }
> +}
> +
> +static Property gicv3_its_props[] = {
> +    DEFINE_PROP_LINK("parent-gicv3", GICv3ITSState, gicv3, "arm-gicv3",
> +                     GICv3State *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void gicv3_its_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    GICv3ITSClass *ic = ARM_GICV3_ITS_CLASS(klass);
> +
> +    dc->realize = gicv3_arm_its_realize;
> +    device_class_set_props(dc, gicv3_its_props);
> +    device_class_set_parent_reset(dc, gicv3_its_reset, &ic->parent_reset);
> +}
> +
> +static const TypeInfo gicv3_its_info = {
> +    .name = TYPE_ARM_GICV3_ITS,
> +    .parent = TYPE_ARM_GICV3_ITS_COMMON,
> +    .instance_size = sizeof(GICv3ITSState),
> +    .class_init = gicv3_its_class_init,
> +    .class_size = sizeof(GICv3ITSClass),
> +};
> +
> +static void gicv3_its_register_types(void)
> +{
> +    type_register_static(&gicv3_its_info);
> +}
> +
> +type_init(gicv3_its_register_types)
> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
> index 66c4c6a188..7d7f3882e7 100644
> --- a/hw/intc/arm_gicv3_its_common.c
> +++ b/hw/intc/arm_gicv3_its_common.c
> @@ -50,6 +50,8 @@ static int gicv3_its_post_load(void *opaque, int version_id)
>  
>  static const VMStateDescription vmstate_its = {
>      .name = "arm_gicv3_its",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
>      .pre_save = gicv3_its_pre_save,
>      .post_load = gicv3_its_post_load,
>      .priority = MIG_PRI_GICV3_ITS,
> @@ -99,14 +101,15 @@ static const MemoryRegionOps gicv3_its_trans_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops)
> +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
> +                         const MemoryRegionOps *tops)
>  {
>      SysBusDevice *sbd = SYS_BUS_DEVICE(s);
>  
>      memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), ops, s,
>                            "control", ITS_CONTROL_SIZE);
>      memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
> -                          &gicv3_its_trans_ops, s,
> +                          tops ? tops : &gicv3_its_trans_ops, s,
>                            "translation", ITS_TRANS_SIZE);
>  
>      /* Our two regions are always adjacent, therefore we now combine them
> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index b554d2ede0..0b4cbed28b 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -106,7 +106,7 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
>      kvm_arm_register_device(&s->iomem_its_cntrl, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
>                              KVM_VGIC_ITS_ADDR_TYPE, s->dev_fd, 0);
>  
> -    gicv3_its_init_mmio(s, NULL);
> +    gicv3_its_init_mmio(s, NULL, NULL);
>  
>      if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
>          GITS_CTLR)) {
> diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
> index 05303a55c8..e0b06930a7 100644
> --- a/hw/intc/gicv3_internal.h
> +++ b/hw/intc/gicv3_internal.h
> @@ -24,6 +24,7 @@
>  #ifndef QEMU_ARM_GICV3_INTERNAL_H
>  #define QEMU_ARM_GICV3_INTERNAL_H
>  
> +#include "hw/registerfields.h"
>  #include "hw/intc/arm_gicv3_common.h"
>  
>  /* Distributor registers, as offsets from the distributor base address */
> @@ -67,6 +68,9 @@
>  #define GICD_CTLR_E1NWF             (1U << 7)
>  #define GICD_CTLR_RWP               (1U << 31)
>  
> +/* 16 bits EventId */
> +#define GICD_TYPER_IDBITS            0xf
> +
>  /*
>   * Redistributor frame offsets from RD_base
>   */
> @@ -122,18 +126,6 @@
>  #define GICR_WAKER_ProcessorSleep    (1U << 1)
>  #define GICR_WAKER_ChildrenAsleep    (1U << 2)
>  
> -#define GICR_PROPBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
> -#define GICR_PROPBASER_ADDR_MASK               (0xfffffffffULL << 12)
> -#define GICR_PROPBASER_SHAREABILITY_MASK       (3U << 10)
> -#define GICR_PROPBASER_CACHEABILITY_MASK       (7U << 7)
> -#define GICR_PROPBASER_IDBITS_MASK             (0x1f)
> -
> -#define GICR_PENDBASER_PTZ                     (1ULL << 62)
> -#define GICR_PENDBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
> -#define GICR_PENDBASER_ADDR_MASK               (0xffffffffULL << 16)
> -#define GICR_PENDBASER_SHAREABILITY_MASK       (3U << 10)
> -#define GICR_PENDBASER_CACHEABILITY_MASK       (7U << 7)
> -
>  #define ICC_CTLR_EL1_CBPR           (1U << 0)
>  #define ICC_CTLR_EL1_EOIMODE        (1U << 1)
>  #define ICC_CTLR_EL1_PMHE           (1U << 6)
> @@ -239,6 +231,78 @@
>  #define ICH_VTR_EL2_PREBITS_SHIFT 26
>  #define ICH_VTR_EL2_PRIBITS_SHIFT 29
>  
> +/* ITS Registers */
> +
> +FIELD(GITS_BASER, SIZE, 0, 8)
> +FIELD(GITS_BASER, PAGESIZE, 8, 2)
> +FIELD(GITS_BASER, SHAREABILITY, 10, 2)
> +FIELD(GITS_BASER, PHYADDR, 12, 36)
> +FIELD(GITS_BASER, PHYADDRL_64K, 16, 32)
> +FIELD(GITS_BASER, PHYADDRH_64K, 48, 4)
> +FIELD(GITS_BASER, ENTRYSIZE, 48, 5)
> +FIELD(GITS_BASER, OUTERCACHE, 53, 3)
> +FIELD(GITS_BASER, TYPE, 56, 3)
> +FIELD(GITS_BASER, INNERCACHE, 59, 3)
> +FIELD(GITS_BASER, INDIRECT, 62, 1)
> +FIELD(GITS_BASER, VALID, 63, 1)
> +
> +FIELD(GITS_CTLR, QUIESCENT, 31, 1)
> +
> +FIELD(GITS_TYPER, PHYSICAL, 0, 1)
> +FIELD(GITS_TYPER, ITT_ENTRY_SIZE, 4, 4)
> +FIELD(GITS_TYPER, IDBITS, 8, 5)
> +FIELD(GITS_TYPER, DEVBITS, 13, 5)
> +FIELD(GITS_TYPER, SEIS, 18, 1)
> +FIELD(GITS_TYPER, PTA, 19, 1)
> +FIELD(GITS_TYPER, CIDBITS, 32, 4)
> +FIELD(GITS_TYPER, CIL, 36, 1)
> +
> +#define GITS_BASER_PAGESIZE_4K                0
> +#define GITS_BASER_PAGESIZE_16K               1
> +#define GITS_BASER_PAGESIZE_64K               2
> +
> +#define GITS_ITT_TYPE_DEVICE                  1ULL
> +#define GITS_ITT_TYPE_COLLECTION              4ULL
> +
> +/**
> + * Default features advertised by this version of ITS
> + */
> +/* Physical LPIs supported */
> +#define GITS_TYPE_PHYSICAL           (1U << 0)
> +
> +/*
> + * 12 bytes Interrupt translation Table Entry size
> + * ITE Lower 8 Bytes
> + * Valid = 1 bit,InterruptType = 1 bit,
> + * Size of LPI number space[considering max 24 bits],
> + * Size of LPI number space[considering max 24 bits],
> + * ITE Higher 4 Bytes
> + * ICID = 16 bits,
> + * vPEID = 16 bits
> + */
> +#define ITS_ITT_ENTRY_SIZE            0xC
> +
> +/* 16 bits EventId */
> +#define ITS_IDBITS                   GICD_TYPER_IDBITS
> +
> +/* 16 bits DeviceId */
> +#define ITS_DEVBITS                   0xF
> +
> +/* 16 bits CollectionId */
> +#define ITS_CIDBITS                  0xF
> +
> +/*
> + * 8 bytes Device Table Entry size
> + * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits
> + */
> +#define GITS_DTE_SIZE                 (0x8ULL)
> +
> +/*
> + * 8 bytes Collection Table Entry size
> + * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE)
> + */
> +#define GITS_CTE_SIZE                 (0x8ULL)
> +
>  /* Special interrupt IDs */
>  #define INTID_SECURE 1020
>  #define INTID_NONSECURE 1021
> diff --git a/hw/intc/meson.build b/hw/intc/meson.build
> index 6e52a166e3..4dcfea6aa8 100644
> --- a/hw/intc/meson.build
> +++ b/hw/intc/meson.build
> @@ -8,6 +8,7 @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: files(
>    'arm_gicv3_dist.c',
>    'arm_gicv3_its_common.c',
>    'arm_gicv3_redist.c',
> +  'arm_gicv3_its.c',
>  ))
>  softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_pic.c'))
>  softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true: files('heathrow_pic.c'))
> diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
> index 5a0952b404..65d1191db1 100644
> --- a/include/hw/intc/arm_gicv3_its_common.h
> +++ b/include/hw/intc/arm_gicv3_its_common.h
> @@ -25,17 +25,22 @@
>  #include "hw/intc/arm_gicv3_common.h"
>  #include "qom/object.h"
>  
> +#define TYPE_ARM_GICV3_ITS "arm-gicv3-its"
> +
>  #define ITS_CONTROL_SIZE 0x10000
>  #define ITS_TRANS_SIZE   0x10000
>  #define ITS_SIZE         (ITS_CONTROL_SIZE + ITS_TRANS_SIZE)
>  
>  #define GITS_CTLR        0x0
>  #define GITS_IIDR        0x4
> +#define GITS_TYPER       0x8
>  #define GITS_CBASER      0x80
>  #define GITS_CWRITER     0x88
>  #define GITS_CREADR      0x90
>  #define GITS_BASER       0x100
>  
> +#define GITS_TRANSLATER  0x0040
> +
>  struct GICv3ITSState {
>      SysBusDevice parent_obj;
>  
> @@ -52,6 +57,7 @@ struct GICv3ITSState {
>      /* Registers */
>      uint32_t ctlr;
>      uint32_t iidr;
> +    uint64_t typer;
>      uint64_t cbaser;
>      uint64_t cwriter;
>      uint64_t creadr;
> @@ -62,7 +68,8 @@ struct GICv3ITSState {
>  
>  typedef struct GICv3ITSState GICv3ITSState;
>  
> -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops);
> +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
> +                   const MemoryRegionOps *tops);
>  
>  #define TYPE_ARM_GICV3_ITS_COMMON "arm-gicv3-its-common"
>  typedef struct GICv3ITSCommonClass GICv3ITSCommonClass;
>
Shashi Mallela July 7, 2021, 2:06 a.m. UTC | #7
On Tue, 2021-07-06 at 09:44 +0200, Eric Auger wrote:
> Hi,
> 
> On 6/30/21 5:31 PM, Shashi Mallela wrote:
> > Added register definitions relevant to ITS,implemented overall
> > ITS device framework with stubs for ITS control and translater
> > regions read/write,extended ITS common to handle mmio init between
> > existing kvm device and newer qemu device.
> > 
> > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> Some of my comments in v4 were not commented nor addressed in v5.
> 
> Also here and in the other respinned patches, please add an
> individual
> history log to track the major changes you made from n-1 to n to help
> the review.
Have addressed all the pending v4 comments and summarized all major
changes in v6 series in the cover-letter section

> Thanks
> 
> Eric
> > ---
> >  hw/intc/arm_gicv3_its.c                | 240
> > +++++++++++++++++++++++++
> >  hw/intc/arm_gicv3_its_common.c         |   7 +-
> >  hw/intc/arm_gicv3_its_kvm.c            |   2 +-
> >  hw/intc/gicv3_internal.h               |  88 +++++++--
> >  hw/intc/meson.build                    |   1 +
> >  include/hw/intc/arm_gicv3_its_common.h |   9 +-
> >  6 files changed, 331 insertions(+), 16 deletions(-)
> >  create mode 100644 hw/intc/arm_gicv3_its.c
> > 
> > diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> > new file mode 100644
> > index 0000000000..545cda3665
> > --- /dev/null
> > +++ b/hw/intc/arm_gicv3_its.c
> > @@ -0,0 +1,240 @@
> > +/*
> > + * ITS emulation for a GICv3-based system
> > + *
> > + * Copyright Linaro.org 2021
> > + *
> > + * Authors:
> > + *  Shashi Mallela <shashi.mallela@linaro.org>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2
> > or (at your
> > + * option) any later version.  See the COPYING file in the top-
> > level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/intc/arm_gicv3_its_common.h"
> > +#include "gicv3_internal.h"
> > +#include "qom/object.h"
> > +
> > +typedef struct GICv3ITSClass GICv3ITSClass;
> > +/* This is reusing the GICv3ITSState typedef from
> > ARM_GICV3_ITS_COMMON */
> > +DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
> > +                     ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
> > +
> > +struct GICv3ITSClass {
> > +    GICv3ITSCommonClass parent_class;
> > +    void (*parent_reset)(DeviceState *dev);
> > +};
> > +
> > +static MemTxResult gicv3_its_translation_write(void *opaque,
> > hwaddr offset,
> > +                                               uint64_t data,
> > unsigned size,
> > +                                               MemTxAttrs attrs)
> > +{
> > +    MemTxResult result = MEMTX_OK;
> > +
> > +    return result;
> > +}
> > +
> > +static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
> > +                              uint64_t value, MemTxAttrs attrs)
> > +{
> > +    MemTxResult result = MEMTX_OK;
> > +
> > +    return result;
> > +}
> > +
> > +static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset,
> > +                             uint64_t *data, MemTxAttrs attrs)
> > +{
> > +    MemTxResult result = MEMTX_OK;
> > +
> > +    return result;
> > +}
> > +
> > +static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
> > +                               uint64_t value, MemTxAttrs attrs)
> > +{
> > +    MemTxResult result = MEMTX_OK;
> > +
> > +    return result;
> > +}
> > +
> > +static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset,
> > +                              uint64_t *data, MemTxAttrs attrs)
> > +{
> > +    MemTxResult result = MEMTX_OK;
> > +
> > +    return result;
> > +}
> > +
> > +static MemTxResult gicv3_its_read(void *opaque, hwaddr offset,
> > uint64_t *data,
> > +                                  unsigned size, MemTxAttrs attrs)
> > +{
> > +    GICv3ITSState *s = (GICv3ITSState *)opaque;
> > +    MemTxResult result;
> > +
> > +    switch (size) {
> > +    case 4:
> > +        result = its_readl(s, offset, data, attrs);
> > +        break;
> > +    case 8:
> > +        result = its_readll(s, offset, data, attrs);
> > +        break;
> > +    default:
> > +        result = MEMTX_ERROR;
> > +        break;
> > +    }
> > +
> > +    if (result == MEMTX_ERROR) {
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +                      "%s: invalid guest read at offset "
> > TARGET_FMT_plx
> > +                      "size %u\n", __func__, offset, size);
> > +        /*
> > +         * The spec requires that reserved registers are RAZ/WI;
> > +         * so use MEMTX_ERROR returns from leaf functions as a way
> > to
> > +         * trigger the guest-error logging but don't return it to
> > +         * the caller, or we'll cause a spurious guest data abort.
> > +         */
> > +        result = MEMTX_OK;
> > +        *data = 0;
> > +    }
> > +    return result;
> > +}
> > +
> > +static MemTxResult gicv3_its_write(void *opaque, hwaddr offset,
> > uint64_t data,
> > +                                   unsigned size, MemTxAttrs
> > attrs)
> > +{
> > +    GICv3ITSState *s = (GICv3ITSState *)opaque;
> > +    MemTxResult result;
> > +
> > +    switch (size) {
> > +    case 4:
> > +        result = its_writel(s, offset, data, attrs);
> > +        break;
> > +    case 8:
> > +        result = its_writell(s, offset, data, attrs);
> > +        break;
> > +    default:
> > +        result = MEMTX_ERROR;
> > +        break;
> > +    }
> > +
> > +    if (result == MEMTX_ERROR) {
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +                      "%s: invalid guest write at offset "
> > TARGET_FMT_plx
> > +                      "size %u\n", __func__, offset, size);
> > +        /*
> > +         * The spec requires that reserved registers are RAZ/WI;
> > +         * so use MEMTX_ERROR returns from leaf functions as a way
> > to
> > +         * trigger the guest-error logging but don't return it to
> > +         * the caller, or we'll cause a spurious guest data abort.
> > +         */
> > +        result = MEMTX_OK;
> > +    }
> > +    return result;
> > +}
> > +
> > +static const MemoryRegionOps gicv3_its_control_ops = {
> > +    .read_with_attrs = gicv3_its_read,
> > +    .write_with_attrs = gicv3_its_write,
> > +    .valid.min_access_size = 4,
> > +    .valid.max_access_size = 8,
> > +    .impl.min_access_size = 4,
> > +    .impl.max_access_size = 8,
> > +    .endianness = DEVICE_NATIVE_ENDIAN,
> > +};
> > +
> > +static const MemoryRegionOps gicv3_its_translation_ops = {
> > +    .write_with_attrs = gicv3_its_translation_write,
> > +    .valid.min_access_size = 2,
> > +    .valid.max_access_size = 4,
> > +    .impl.min_access_size = 2,
> > +    .impl.max_access_size = 4,
> > +    .endianness = DEVICE_NATIVE_ENDIAN,
> > +};
> > +
> > +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
> > +{
> > +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> > +
> > +    gicv3_its_init_mmio(s, &gicv3_its_control_ops,
> > &gicv3_its_translation_ops);
> > +
> > +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> > +        /* set the ITS default features supported */
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL,
> > +                              GITS_TYPE_PHYSICAL);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER,
> > ITT_ENTRY_SIZE,
> > +                              ITS_ITT_ENTRY_SIZE - 1);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS,
> > ITS_IDBITS);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, DEVBITS,
> > ITS_DEVBITS);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIL, 1);
> > +        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIDBITS,
> > ITS_CIDBITS);
> > +    }
> > +}
> > +
> > +static void gicv3_its_reset(DeviceState *dev)
> > +{
> > +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> > +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> > +
> > +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> > +        c->parent_reset(dev);
> > +
> > +        /* Quiescent bit reset to 1 */
> > +        s->ctlr = FIELD_DP32(s->ctlr, GITS_CTLR, QUIESCENT, 1);
> > +
> > +        /*
> > +         * setting GITS_BASER0.Type = 0b001 (Device)
> > +         *         GITS_BASER1.Type = 0b100 (Collection Table)
> > +         *         GITS_BASER<n>.Type,where n = 3 to 7 are 0b00
> > (Unimplemented)
> > +         *         GITS_BASER<0,1>.Page_Size = 64KB
> > +         * and default translation table entry size to 16 bytes
> > +         */
> > +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, TYPE,
> > +                                 GITS_ITT_TYPE_DEVICE);
> > +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER,
> > PAGESIZE,
> > +                                 GITS_BASER_PAGESIZE_64K);
> > +        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER,
> > ENTRYSIZE,
> > +                                 GITS_DTE_SIZE - 1);
> > +
> > +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, TYPE,
> > +                                 GITS_ITT_TYPE_COLLECTION);
> > +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER,
> > PAGESIZE,
> > +                                 GITS_BASER_PAGESIZE_64K);
> > +        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER,
> > ENTRYSIZE,
> > +                                 GITS_CTE_SIZE - 1);
> > +    }
> > +}
> > +
> > +static Property gicv3_its_props[] = {
> > +    DEFINE_PROP_LINK("parent-gicv3", GICv3ITSState, gicv3, "arm-
> > gicv3",
> > +                     GICv3State *),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void gicv3_its_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    GICv3ITSClass *ic = ARM_GICV3_ITS_CLASS(klass);
> > +
> > +    dc->realize = gicv3_arm_its_realize;
> > +    device_class_set_props(dc, gicv3_its_props);
> > +    device_class_set_parent_reset(dc, gicv3_its_reset, &ic-
> > >parent_reset);
> > +}
> > +
> > +static const TypeInfo gicv3_its_info = {
> > +    .name = TYPE_ARM_GICV3_ITS,
> > +    .parent = TYPE_ARM_GICV3_ITS_COMMON,
> > +    .instance_size = sizeof(GICv3ITSState),
> > +    .class_init = gicv3_its_class_init,
> > +    .class_size = sizeof(GICv3ITSClass),
> > +};
> > +
> > +static void gicv3_its_register_types(void)
> > +{
> > +    type_register_static(&gicv3_its_info);
> > +}
> > +
> > +type_init(gicv3_its_register_types)
> > diff --git a/hw/intc/arm_gicv3_its_common.c
> > b/hw/intc/arm_gicv3_its_common.c
> > index 66c4c6a188..7d7f3882e7 100644
> > --- a/hw/intc/arm_gicv3_its_common.c
> > +++ b/hw/intc/arm_gicv3_its_common.c
> > @@ -50,6 +50,8 @@ static int gicv3_its_post_load(void *opaque, int
> > version_id)
> >  
> >  static const VMStateDescription vmstate_its = {
> >      .name = "arm_gicv3_its",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> >      .pre_save = gicv3_its_pre_save,
> >      .post_load = gicv3_its_post_load,
> >      .priority = MIG_PRI_GICV3_ITS,
> > @@ -99,14 +101,15 @@ static const MemoryRegionOps
> > gicv3_its_trans_ops = {
> >      .endianness = DEVICE_NATIVE_ENDIAN,
> >  };
> >  
> > -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps
> > *ops)
> > +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps
> > *ops,
> > +                         const MemoryRegionOps *tops)
> >  {
> >      SysBusDevice *sbd = SYS_BUS_DEVICE(s);
> >  
> >      memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), ops, s,
> >                            "control", ITS_CONTROL_SIZE);
> >      memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
> > -                          &gicv3_its_trans_ops, s,
> > +                          tops ? tops : &gicv3_its_trans_ops, s,
> >                            "translation", ITS_TRANS_SIZE);
> >  
> >      /* Our two regions are always adjacent, therefore we now
> > combine them
> > diff --git a/hw/intc/arm_gicv3_its_kvm.c
> > b/hw/intc/arm_gicv3_its_kvm.c
> > index b554d2ede0..0b4cbed28b 100644
> > --- a/hw/intc/arm_gicv3_its_kvm.c
> > +++ b/hw/intc/arm_gicv3_its_kvm.c
> > @@ -106,7 +106,7 @@ static void kvm_arm_its_realize(DeviceState
> > *dev, Error **errp)
> >      kvm_arm_register_device(&s->iomem_its_cntrl, -1,
> > KVM_DEV_ARM_VGIC_GRP_ADDR,
> >                              KVM_VGIC_ITS_ADDR_TYPE, s->dev_fd, 0);
> >  
> > -    gicv3_its_init_mmio(s, NULL);
> > +    gicv3_its_init_mmio(s, NULL, NULL);
> >  
> >      if (!kvm_device_check_attr(s->dev_fd,
> > KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> >          GITS_CTLR)) {
> > diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
> > index 05303a55c8..e0b06930a7 100644
> > --- a/hw/intc/gicv3_internal.h
> > +++ b/hw/intc/gicv3_internal.h
> > @@ -24,6 +24,7 @@
> >  #ifndef QEMU_ARM_GICV3_INTERNAL_H
> >  #define QEMU_ARM_GICV3_INTERNAL_H
> >  
> > +#include "hw/registerfields.h"
> >  #include "hw/intc/arm_gicv3_common.h"
> >  
> >  /* Distributor registers, as offsets from the distributor base
> > address */
> > @@ -67,6 +68,9 @@
> >  #define GICD_CTLR_E1NWF             (1U << 7)
> >  #define GICD_CTLR_RWP               (1U << 31)
> >  
> > +/* 16 bits EventId */
> > +#define GICD_TYPER_IDBITS            0xf
> > +
> >  /*
> >   * Redistributor frame offsets from RD_base
> >   */
> > @@ -122,18 +126,6 @@
> >  #define GICR_WAKER_ProcessorSleep    (1U << 1)
> >  #define GICR_WAKER_ChildrenAsleep    (1U << 2)
> >  
> > -#define GICR_PROPBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
> > -#define GICR_PROPBASER_ADDR_MASK               (0xfffffffffULL <<
> > 12)
> > -#define GICR_PROPBASER_SHAREABILITY_MASK       (3U << 10)
> > -#define GICR_PROPBASER_CACHEABILITY_MASK       (7U << 7)
> > -#define GICR_PROPBASER_IDBITS_MASK             (0x1f)
> > -
> > -#define GICR_PENDBASER_PTZ                     (1ULL << 62)
> > -#define GICR_PENDBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
> > -#define GICR_PENDBASER_ADDR_MASK               (0xffffffffULL <<
> > 16)
> > -#define GICR_PENDBASER_SHAREABILITY_MASK       (3U << 10)
> > -#define GICR_PENDBASER_CACHEABILITY_MASK       (7U << 7)
> > -
> >  #define ICC_CTLR_EL1_CBPR           (1U << 0)
> >  #define ICC_CTLR_EL1_EOIMODE        (1U << 1)
> >  #define ICC_CTLR_EL1_PMHE           (1U << 6)
> > @@ -239,6 +231,78 @@
> >  #define ICH_VTR_EL2_PREBITS_SHIFT 26
> >  #define ICH_VTR_EL2_PRIBITS_SHIFT 29
> >  
> > +/* ITS Registers */
> > +
> > +FIELD(GITS_BASER, SIZE, 0, 8)
> > +FIELD(GITS_BASER, PAGESIZE, 8, 2)
> > +FIELD(GITS_BASER, SHAREABILITY, 10, 2)
> > +FIELD(GITS_BASER, PHYADDR, 12, 36)
> > +FIELD(GITS_BASER, PHYADDRL_64K, 16, 32)
> > +FIELD(GITS_BASER, PHYADDRH_64K, 48, 4)
> > +FIELD(GITS_BASER, ENTRYSIZE, 48, 5)
> > +FIELD(GITS_BASER, OUTERCACHE, 53, 3)
> > +FIELD(GITS_BASER, TYPE, 56, 3)
> > +FIELD(GITS_BASER, INNERCACHE, 59, 3)
> > +FIELD(GITS_BASER, INDIRECT, 62, 1)
> > +FIELD(GITS_BASER, VALID, 63, 1)
> > +
> > +FIELD(GITS_CTLR, QUIESCENT, 31, 1)
> > +
> > +FIELD(GITS_TYPER, PHYSICAL, 0, 1)
> > +FIELD(GITS_TYPER, ITT_ENTRY_SIZE, 4, 4)
> > +FIELD(GITS_TYPER, IDBITS, 8, 5)
> > +FIELD(GITS_TYPER, DEVBITS, 13, 5)
> > +FIELD(GITS_TYPER, SEIS, 18, 1)
> > +FIELD(GITS_TYPER, PTA, 19, 1)
> > +FIELD(GITS_TYPER, CIDBITS, 32, 4)
> > +FIELD(GITS_TYPER, CIL, 36, 1)
> > +
> > +#define GITS_BASER_PAGESIZE_4K                0
> > +#define GITS_BASER_PAGESIZE_16K               1
> > +#define GITS_BASER_PAGESIZE_64K               2
> > +
> > +#define GITS_ITT_TYPE_DEVICE                  1ULL
> > +#define GITS_ITT_TYPE_COLLECTION              4ULL
> > +
> > +/**
> > + * Default features advertised by this version of ITS
> > + */
> > +/* Physical LPIs supported */
> > +#define GITS_TYPE_PHYSICAL           (1U << 0)
> > +
> > +/*
> > + * 12 bytes Interrupt translation Table Entry size
> > + * ITE Lower 8 Bytes
> > + * Valid = 1 bit,InterruptType = 1 bit,
> > + * Size of LPI number space[considering max 24 bits],
> > + * Size of LPI number space[considering max 24 bits],
> > + * ITE Higher 4 Bytes
> > + * ICID = 16 bits,
> > + * vPEID = 16 bits
> > + */
> > +#define ITS_ITT_ENTRY_SIZE            0xC
> > +
> > +/* 16 bits EventId */
> > +#define ITS_IDBITS                   GICD_TYPER_IDBITS
> > +
> > +/* 16 bits DeviceId */
> > +#define ITS_DEVBITS                   0xF
> > +
> > +/* 16 bits CollectionId */
> > +#define ITS_CIDBITS                  0xF
> > +
> > +/*
> > + * 8 bytes Device Table Entry size
> > + * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits
> > + */
> > +#define GITS_DTE_SIZE                 (0x8ULL)
> > +
> > +/*
> > + * 8 bytes Collection Table Entry size
> > + * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE)
> > + */
> > +#define GITS_CTE_SIZE                 (0x8ULL)
> > +
> >  /* Special interrupt IDs */
> >  #define INTID_SECURE 1020
> >  #define INTID_NONSECURE 1021
> > diff --git a/hw/intc/meson.build b/hw/intc/meson.build
> > index 6e52a166e3..4dcfea6aa8 100644
> > --- a/hw/intc/meson.build
> > +++ b/hw/intc/meson.build
> > @@ -8,6 +8,7 @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true:
> > files(
> >    'arm_gicv3_dist.c',
> >    'arm_gicv3_its_common.c',
> >    'arm_gicv3_redist.c',
> > +  'arm_gicv3_its.c',
> >  ))
> >  softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true:
> > files('etraxfs_pic.c'))
> >  softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true:
> > files('heathrow_pic.c'))
> > diff --git a/include/hw/intc/arm_gicv3_its_common.h
> > b/include/hw/intc/arm_gicv3_its_common.h
> > index 5a0952b404..65d1191db1 100644
> > --- a/include/hw/intc/arm_gicv3_its_common.h
> > +++ b/include/hw/intc/arm_gicv3_its_common.h
> > @@ -25,17 +25,22 @@
> >  #include "hw/intc/arm_gicv3_common.h"
> >  #include "qom/object.h"
> >  
> > +#define TYPE_ARM_GICV3_ITS "arm-gicv3-its"
> > +
> >  #define ITS_CONTROL_SIZE 0x10000
> >  #define ITS_TRANS_SIZE   0x10000
> >  #define ITS_SIZE         (ITS_CONTROL_SIZE + ITS_TRANS_SIZE)
> >  
> >  #define GITS_CTLR        0x0
> >  #define GITS_IIDR        0x4
> > +#define GITS_TYPER       0x8
> >  #define GITS_CBASER      0x80
> >  #define GITS_CWRITER     0x88
> >  #define GITS_CREADR      0x90
> >  #define GITS_BASER       0x100
> >  
> > +#define GITS_TRANSLATER  0x0040
> > +
> >  struct GICv3ITSState {
> >      SysBusDevice parent_obj;
> >  
> > @@ -52,6 +57,7 @@ struct GICv3ITSState {
> >      /* Registers */
> >      uint32_t ctlr;
> >      uint32_t iidr;
> > +    uint64_t typer;
> >      uint64_t cbaser;
> >      uint64_t cwriter;
> >      uint64_t creadr;
> > @@ -62,7 +68,8 @@ struct GICv3ITSState {
> >  
> >  typedef struct GICv3ITSState GICv3ITSState;
> >  
> > -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps
> > *ops);
> > +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps
> > *ops,
> > +                   const MemoryRegionOps *tops);
> >  
> >  #define TYPE_ARM_GICV3_ITS_COMMON "arm-gicv3-its-common"
> >  typedef struct GICv3ITSCommonClass GICv3ITSCommonClass;
> >
Shashi Mallela July 7, 2021, 2:08 a.m. UTC | #8
On Mon, 2021-07-05 at 19:58 +0100, Peter Maydell wrote:
> On Mon, 5 Jul 2021 at 18:04, <shashi.mallela@linaro.org> wrote:
> > On Mon, 2021-07-05 at 17:25 +0100, Peter Maydell wrote:
> > > On Mon, 5 Jul 2021 at 16:55, <shashi.mallela@linaro.org> wrote:
> > > > On Mon, 2021-07-05 at 15:58 +0100, Peter Maydell wrote:
> > > > > On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <
> > > > > shashi.mallela@linaro.org> wrote:
> > > > > > Added register definitions relevant to ITS,implemented
> > > > > > overall
> > > > > > ITS device framework with stubs for ITS control and
> > > > > > translater
> > > > > > regions read/write,extended ITS common to handle mmio init
> > > > > > between
> > > > > > existing kvm device and newer qemu device.
> > > > > > 
> > > > > > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> > > > > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > > > > > +static void gicv3_arm_its_realize(DeviceState *dev, Error
> > > > > > **errp)
> > > > > > +{
> > > > > > +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> > > > > > +
> > > > > > +    gicv3_its_init_mmio(s, &gicv3_its_control_ops,
> > > > > > &gicv3_its_translation_ops);
> > > > > > +
> > > > > > +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> > > > > 
> > > > > Can you remind me why we make this check, please? When would
> > > > > we
> > > > > have created an ITS device but not have a GICv3 with LPI
> > > > > support?
> > > > This check applies to GIC's physical LPI support only as
> > > > against
> > > > GIC's
> > > > virtual LPI support.
> > > 
> > > Right, but when would we have a GIC with no physical LPI support
> > > but an ITS is present ?
> > If we only support Direct injection of virtual interrupts (which
> > can
> > have their own vPEID and the vPE table),then the ITS present could
> > havejust virtual LPI support
> 
> This patchset does not support a virtual-LPI-only ITS, though:
> it doesn't support virtual LPIs at all.
> If you use it with CPUs without physical LPI support , this code will
> skip
> entirely setting GITS_TYPER and will make reset do nothing, and then
> the
> rest of the ITS implementation will misbehave.
> 
> I think what we should do is:
>  * in realize, check every CPU to make sure its redistributor
>    supports physical LPIs, and return an error from realize if not
>  * in reset, don't check anything
Done
> 
> If we add virtual-LPI-only ITS support later, we can always update
> this code appropriately.
> 
> thanks
> -- PMM
diff mbox series

Patch

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
new file mode 100644
index 0000000000..545cda3665
--- /dev/null
+++ b/hw/intc/arm_gicv3_its.c
@@ -0,0 +1,240 @@ 
+/*
+ * ITS emulation for a GICv3-based system
+ *
+ * Copyright Linaro.org 2021
+ *
+ * Authors:
+ *  Shashi Mallela <shashi.mallela@linaro.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version.  See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/qdev-properties.h"
+#include "hw/intc/arm_gicv3_its_common.h"
+#include "gicv3_internal.h"
+#include "qom/object.h"
+
+typedef struct GICv3ITSClass GICv3ITSClass;
+/* This is reusing the GICv3ITSState typedef from ARM_GICV3_ITS_COMMON */
+DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
+                     ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
+
+struct GICv3ITSClass {
+    GICv3ITSCommonClass parent_class;
+    void (*parent_reset)(DeviceState *dev);
+};
+
+static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset,
+                                               uint64_t data, unsigned size,
+                                               MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
+                              uint64_t value, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset,
+                             uint64_t *data, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
+                               uint64_t value, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset,
+                              uint64_t *data, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult gicv3_its_read(void *opaque, hwaddr offset, uint64_t *data,
+                                  unsigned size, MemTxAttrs attrs)
+{
+    GICv3ITSState *s = (GICv3ITSState *)opaque;
+    MemTxResult result;
+
+    switch (size) {
+    case 4:
+        result = its_readl(s, offset, data, attrs);
+        break;
+    case 8:
+        result = its_readll(s, offset, data, attrs);
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
+
+    if (result == MEMTX_ERROR) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid guest read at offset " TARGET_FMT_plx
+                      "size %u\n", __func__, offset, size);
+        /*
+         * The spec requires that reserved registers are RAZ/WI;
+         * so use MEMTX_ERROR returns from leaf functions as a way to
+         * trigger the guest-error logging but don't return it to
+         * the caller, or we'll cause a spurious guest data abort.
+         */
+        result = MEMTX_OK;
+        *data = 0;
+    }
+    return result;
+}
+
+static MemTxResult gicv3_its_write(void *opaque, hwaddr offset, uint64_t data,
+                                   unsigned size, MemTxAttrs attrs)
+{
+    GICv3ITSState *s = (GICv3ITSState *)opaque;
+    MemTxResult result;
+
+    switch (size) {
+    case 4:
+        result = its_writel(s, offset, data, attrs);
+        break;
+    case 8:
+        result = its_writell(s, offset, data, attrs);
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
+
+    if (result == MEMTX_ERROR) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid guest write at offset " TARGET_FMT_plx
+                      "size %u\n", __func__, offset, size);
+        /*
+         * The spec requires that reserved registers are RAZ/WI;
+         * so use MEMTX_ERROR returns from leaf functions as a way to
+         * trigger the guest-error logging but don't return it to
+         * the caller, or we'll cause a spurious guest data abort.
+         */
+        result = MEMTX_OK;
+    }
+    return result;
+}
+
+static const MemoryRegionOps gicv3_its_control_ops = {
+    .read_with_attrs = gicv3_its_read,
+    .write_with_attrs = gicv3_its_write,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 8,
+    .impl.min_access_size = 4,
+    .impl.max_access_size = 8,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static const MemoryRegionOps gicv3_its_translation_ops = {
+    .write_with_attrs = gicv3_its_translation_write,
+    .valid.min_access_size = 2,
+    .valid.max_access_size = 4,
+    .impl.min_access_size = 2,
+    .impl.max_access_size = 4,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
+{
+    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
+
+    gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_translation_ops);
+
+    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
+        /* set the ITS default features supported */
+        s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL,
+                              GITS_TYPE_PHYSICAL);
+        s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE,
+                              ITS_ITT_ENTRY_SIZE - 1);
+        s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, ITS_IDBITS);
+        s->typer = FIELD_DP64(s->typer, GITS_TYPER, DEVBITS, ITS_DEVBITS);
+        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIL, 1);
+        s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIDBITS, ITS_CIDBITS);
+    }
+}
+
+static void gicv3_its_reset(DeviceState *dev)
+{
+    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+
+    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
+        c->parent_reset(dev);
+
+        /* Quiescent bit reset to 1 */
+        s->ctlr = FIELD_DP32(s->ctlr, GITS_CTLR, QUIESCENT, 1);
+
+        /*
+         * setting GITS_BASER0.Type = 0b001 (Device)
+         *         GITS_BASER1.Type = 0b100 (Collection Table)
+         *         GITS_BASER<n>.Type,where n = 3 to 7 are 0b00 (Unimplemented)
+         *         GITS_BASER<0,1>.Page_Size = 64KB
+         * and default translation table entry size to 16 bytes
+         */
+        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, TYPE,
+                                 GITS_ITT_TYPE_DEVICE);
+        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, PAGESIZE,
+                                 GITS_BASER_PAGESIZE_64K);
+        s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, ENTRYSIZE,
+                                 GITS_DTE_SIZE - 1);
+
+        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, TYPE,
+                                 GITS_ITT_TYPE_COLLECTION);
+        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, PAGESIZE,
+                                 GITS_BASER_PAGESIZE_64K);
+        s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, ENTRYSIZE,
+                                 GITS_CTE_SIZE - 1);
+    }
+}
+
+static Property gicv3_its_props[] = {
+    DEFINE_PROP_LINK("parent-gicv3", GICv3ITSState, gicv3, "arm-gicv3",
+                     GICv3State *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void gicv3_its_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    GICv3ITSClass *ic = ARM_GICV3_ITS_CLASS(klass);
+
+    dc->realize = gicv3_arm_its_realize;
+    device_class_set_props(dc, gicv3_its_props);
+    device_class_set_parent_reset(dc, gicv3_its_reset, &ic->parent_reset);
+}
+
+static const TypeInfo gicv3_its_info = {
+    .name = TYPE_ARM_GICV3_ITS,
+    .parent = TYPE_ARM_GICV3_ITS_COMMON,
+    .instance_size = sizeof(GICv3ITSState),
+    .class_init = gicv3_its_class_init,
+    .class_size = sizeof(GICv3ITSClass),
+};
+
+static void gicv3_its_register_types(void)
+{
+    type_register_static(&gicv3_its_info);
+}
+
+type_init(gicv3_its_register_types)
diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
index 66c4c6a188..7d7f3882e7 100644
--- a/hw/intc/arm_gicv3_its_common.c
+++ b/hw/intc/arm_gicv3_its_common.c
@@ -50,6 +50,8 @@  static int gicv3_its_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_its = {
     .name = "arm_gicv3_its",
+    .version_id = 1,
+    .minimum_version_id = 1,
     .pre_save = gicv3_its_pre_save,
     .post_load = gicv3_its_post_load,
     .priority = MIG_PRI_GICV3_ITS,
@@ -99,14 +101,15 @@  static const MemoryRegionOps gicv3_its_trans_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops)
+void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
+                         const MemoryRegionOps *tops)
 {
     SysBusDevice *sbd = SYS_BUS_DEVICE(s);
 
     memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), ops, s,
                           "control", ITS_CONTROL_SIZE);
     memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
-                          &gicv3_its_trans_ops, s,
+                          tops ? tops : &gicv3_its_trans_ops, s,
                           "translation", ITS_TRANS_SIZE);
 
     /* Our two regions are always adjacent, therefore we now combine them
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index b554d2ede0..0b4cbed28b 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -106,7 +106,7 @@  static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
     kvm_arm_register_device(&s->iomem_its_cntrl, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
                             KVM_VGIC_ITS_ADDR_TYPE, s->dev_fd, 0);
 
-    gicv3_its_init_mmio(s, NULL);
+    gicv3_its_init_mmio(s, NULL, NULL);
 
     if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
         GITS_CTLR)) {
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 05303a55c8..e0b06930a7 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -24,6 +24,7 @@ 
 #ifndef QEMU_ARM_GICV3_INTERNAL_H
 #define QEMU_ARM_GICV3_INTERNAL_H
 
+#include "hw/registerfields.h"
 #include "hw/intc/arm_gicv3_common.h"
 
 /* Distributor registers, as offsets from the distributor base address */
@@ -67,6 +68,9 @@ 
 #define GICD_CTLR_E1NWF             (1U << 7)
 #define GICD_CTLR_RWP               (1U << 31)
 
+/* 16 bits EventId */
+#define GICD_TYPER_IDBITS            0xf
+
 /*
  * Redistributor frame offsets from RD_base
  */
@@ -122,18 +126,6 @@ 
 #define GICR_WAKER_ProcessorSleep    (1U << 1)
 #define GICR_WAKER_ChildrenAsleep    (1U << 2)
 
-#define GICR_PROPBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
-#define GICR_PROPBASER_ADDR_MASK               (0xfffffffffULL << 12)
-#define GICR_PROPBASER_SHAREABILITY_MASK       (3U << 10)
-#define GICR_PROPBASER_CACHEABILITY_MASK       (7U << 7)
-#define GICR_PROPBASER_IDBITS_MASK             (0x1f)
-
-#define GICR_PENDBASER_PTZ                     (1ULL << 62)
-#define GICR_PENDBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
-#define GICR_PENDBASER_ADDR_MASK               (0xffffffffULL << 16)
-#define GICR_PENDBASER_SHAREABILITY_MASK       (3U << 10)
-#define GICR_PENDBASER_CACHEABILITY_MASK       (7U << 7)
-
 #define ICC_CTLR_EL1_CBPR           (1U << 0)
 #define ICC_CTLR_EL1_EOIMODE        (1U << 1)
 #define ICC_CTLR_EL1_PMHE           (1U << 6)
@@ -239,6 +231,78 @@ 
 #define ICH_VTR_EL2_PREBITS_SHIFT 26
 #define ICH_VTR_EL2_PRIBITS_SHIFT 29
 
+/* ITS Registers */
+
+FIELD(GITS_BASER, SIZE, 0, 8)
+FIELD(GITS_BASER, PAGESIZE, 8, 2)
+FIELD(GITS_BASER, SHAREABILITY, 10, 2)
+FIELD(GITS_BASER, PHYADDR, 12, 36)
+FIELD(GITS_BASER, PHYADDRL_64K, 16, 32)
+FIELD(GITS_BASER, PHYADDRH_64K, 48, 4)
+FIELD(GITS_BASER, ENTRYSIZE, 48, 5)
+FIELD(GITS_BASER, OUTERCACHE, 53, 3)
+FIELD(GITS_BASER, TYPE, 56, 3)
+FIELD(GITS_BASER, INNERCACHE, 59, 3)
+FIELD(GITS_BASER, INDIRECT, 62, 1)
+FIELD(GITS_BASER, VALID, 63, 1)
+
+FIELD(GITS_CTLR, QUIESCENT, 31, 1)
+
+FIELD(GITS_TYPER, PHYSICAL, 0, 1)
+FIELD(GITS_TYPER, ITT_ENTRY_SIZE, 4, 4)
+FIELD(GITS_TYPER, IDBITS, 8, 5)
+FIELD(GITS_TYPER, DEVBITS, 13, 5)
+FIELD(GITS_TYPER, SEIS, 18, 1)
+FIELD(GITS_TYPER, PTA, 19, 1)
+FIELD(GITS_TYPER, CIDBITS, 32, 4)
+FIELD(GITS_TYPER, CIL, 36, 1)
+
+#define GITS_BASER_PAGESIZE_4K                0
+#define GITS_BASER_PAGESIZE_16K               1
+#define GITS_BASER_PAGESIZE_64K               2
+
+#define GITS_ITT_TYPE_DEVICE                  1ULL
+#define GITS_ITT_TYPE_COLLECTION              4ULL
+
+/**
+ * Default features advertised by this version of ITS
+ */
+/* Physical LPIs supported */
+#define GITS_TYPE_PHYSICAL           (1U << 0)
+
+/*
+ * 12 bytes Interrupt translation Table Entry size
+ * ITE Lower 8 Bytes
+ * Valid = 1 bit,InterruptType = 1 bit,
+ * Size of LPI number space[considering max 24 bits],
+ * Size of LPI number space[considering max 24 bits],
+ * ITE Higher 4 Bytes
+ * ICID = 16 bits,
+ * vPEID = 16 bits
+ */
+#define ITS_ITT_ENTRY_SIZE            0xC
+
+/* 16 bits EventId */
+#define ITS_IDBITS                   GICD_TYPER_IDBITS
+
+/* 16 bits DeviceId */
+#define ITS_DEVBITS                   0xF
+
+/* 16 bits CollectionId */
+#define ITS_CIDBITS                  0xF
+
+/*
+ * 8 bytes Device Table Entry size
+ * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits
+ */
+#define GITS_DTE_SIZE                 (0x8ULL)
+
+/*
+ * 8 bytes Collection Table Entry size
+ * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE)
+ */
+#define GITS_CTE_SIZE                 (0x8ULL)
+
 /* Special interrupt IDs */
 #define INTID_SECURE 1020
 #define INTID_NONSECURE 1021
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 6e52a166e3..4dcfea6aa8 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -8,6 +8,7 @@  softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: files(
   'arm_gicv3_dist.c',
   'arm_gicv3_its_common.c',
   'arm_gicv3_redist.c',
+  'arm_gicv3_its.c',
 ))
 softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_pic.c'))
 softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true: files('heathrow_pic.c'))
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
index 5a0952b404..65d1191db1 100644
--- a/include/hw/intc/arm_gicv3_its_common.h
+++ b/include/hw/intc/arm_gicv3_its_common.h
@@ -25,17 +25,22 @@ 
 #include "hw/intc/arm_gicv3_common.h"
 #include "qom/object.h"
 
+#define TYPE_ARM_GICV3_ITS "arm-gicv3-its"
+
 #define ITS_CONTROL_SIZE 0x10000
 #define ITS_TRANS_SIZE   0x10000
 #define ITS_SIZE         (ITS_CONTROL_SIZE + ITS_TRANS_SIZE)
 
 #define GITS_CTLR        0x0
 #define GITS_IIDR        0x4
+#define GITS_TYPER       0x8
 #define GITS_CBASER      0x80
 #define GITS_CWRITER     0x88
 #define GITS_CREADR      0x90
 #define GITS_BASER       0x100
 
+#define GITS_TRANSLATER  0x0040
+
 struct GICv3ITSState {
     SysBusDevice parent_obj;
 
@@ -52,6 +57,7 @@  struct GICv3ITSState {
     /* Registers */
     uint32_t ctlr;
     uint32_t iidr;
+    uint64_t typer;
     uint64_t cbaser;
     uint64_t cwriter;
     uint64_t creadr;
@@ -62,7 +68,8 @@  struct GICv3ITSState {
 
 typedef struct GICv3ITSState GICv3ITSState;
 
-void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops);
+void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
+                   const MemoryRegionOps *tops);
 
 #define TYPE_ARM_GICV3_ITS_COMMON "arm-gicv3-its-common"
 typedef struct GICv3ITSCommonClass GICv3ITSCommonClass;