Re: [PATCH v5 04/10] hw/intc: GICv3 ITS Command processing

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 6 Jul 2021 at 13:46, <shashi.mallela@linaro.org> wrote:
>
> On Tue, 2021-07-06 at 10:19 +0100, Peter Maydell wrote:
> > On Tue, 6 Jul 2021 at 04:25, <shashi.mallela@linaro.org> wrote:
> >
> > But the pseudocode for MAPTI does not say anywhere that we should
> > be checking the pIntID against any CPU's GICR_PROPBASER field.
> > It is checked only by the checks in LPIOutOfRange(), which tests:
> >  * is it larger than permitted by GICD_TYPER.IDbits
> >  * is it not in the LPI range and not 1023
> >
> > Checking whether the intID is too big and would cause us to index
> > off the end of the redistributor's configuration table should be done
> > later, only when the ITS actually sends the interrupt to a particular
> > redistributor, I think.
> >
> > (You can't rely on the guest having done the MAPC before the MAPTI;
> > and in any case the guest could choose to do a MAPC to a different
> > redistributor after it's done the MAPTI.)

> We already have the "intID too big check" in place within the
> redistributor processing when ITS sends the interrupt trigger.
> "the LPI range and not 1023" is also handled in this function,but for
> validating "is it larger than permitted by GICD_TYPER.IDbits",the
> source of GICD_TYPER.IDbits is GICR_PROPBASER because we pick up min of
> GICR_PROPBASER.IDbits and GICD_TYPER.IDBits.
>
> If we are to not use gicr_propbaser,then are we good to just accept the
> intID value here since we are validating the same during interrupt
> processing?

You should check the things the pseudocode says you should check.
When processing MAPTI, that's GICD_TYPER.IDbits. GICR_PROPBASER.IDbits
is not the same thing because the guest can set it to a smaller value.

thanks
-- PMM