On Wed, 30 Jun 2021 at 16:32, Shashi Mallela <shashi.mall...@linaro.org> wrote: > > Added ITS command queue handling for MAPTI,MAPI commands,handled ITS > translation which triggers an LPI via INT command as well as write > to GITS_TRANSLATER register,defined enum to differentiate between ITS > command interrupt trigger and GITS_TRANSLATER based interrupt trigger. > Each of these commands make use of other functionalities implemented to > get device table entry,collection table entry or interrupt translation > table entry required for their processing. > > Signed-off-by: Shashi Mallela <shashi.mall...@linaro.org> > ---
> +static MemTxResult process_mapti(GICv3ITSState *s, uint64_t value, > + uint32_t offset, bool ignore_pInt) > +{ > + AddressSpace *as = &s->gicv3->dma_as; > + uint32_t devid, eventid; > + uint32_t pIntid = 0; > + uint32_t max_eventid, max_Intid; > + bool dte_valid; > + MemTxResult res = MEMTX_OK; > + uint16_t icid = 0; > + uint64_t dte = 0; > + IteEntry ite; > + uint32_t int_spurious = INTID_SPURIOUS; > + uint64_t idbits; > + > + devid = ((value & DEVID_MASK) >> DEVID_SHIFT); > + offset += NUM_BYTES_IN_DW; > + value = address_space_ldq_le(as, s->cq.base_addr + offset, > + MEMTXATTRS_UNSPECIFIED, &res); > + > + if (res != MEMTX_OK) { > + return res; > + } > + > + eventid = (value & EVENTID_MASK); > + > + if (!ignore_pInt) { > + pIntid = ((value & pINTID_MASK) >> pINTID_SHIFT); > + } > + > + offset += NUM_BYTES_IN_DW; > + value = address_space_ldq_le(as, s->cq.base_addr + offset, > + MEMTXATTRS_UNSPECIFIED, &res); > + > + if (res != MEMTX_OK) { > + return res; > + } > + > + icid = value & ICID_MASK; > + > + dte = get_dte(s, devid, &res); > + > + if (res != MEMTX_OK) { > + return res; > + } > + dte_valid = dte & TABLE_ENTRY_VALID_MASK; > + > + max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1)); > + > + if (!ignore_pInt) { > + idbits = MIN(FIELD_EX64(s->gicv3->cpu->gicr_propbaser, > GICR_PROPBASER, > + IDBITS), GICD_TYPER_IDBITS); I missed this the first time around, but I don't think this is right. Different CPUs could have different GICR_PROPBASER values, so checking against just one of them is wrong. The pseudocode only tests LPIOutOfRange() which is documented as testing "larger than GICD_TYPER.IDbits or not in the LPI range and not 1023". So I don't think we should be looking at the GICR_PROPBASER field here. More generally, "s->gicv3->cpu->something" is usually going to be wrong, because it is implicitly looking at CPU 0; often either there should be something else telling is which CPU to use (as in &s->gicv3->cpu[rdbase] where the CTE told us which redistributor), or we might need to operate on all CPUs/redistributors. The only exception is where we can guarantee that all the CPUs are the same (eg when looking at GICR_TYPER.PLPIS.) thanks -- PMM