On Mon, 2021-07-05 at 15:58 +0100, Peter Maydell wrote:
> On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <
> shashi.mall...@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.mall...@linaro.org>
> > Reviewed-by: Peter Maydell <peter.mayd...@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
