[ ... ]
>>> +void pnv_psi_irq_set(PnvPsi *psi, PnvPsiIrq irq, bool state)
>>> +{
>>> + ICSState *ics = &psi->ics;
>>> + uint32_t xivr_reg;
>>> + uint32_t stat_reg;
>>> + uint64_t stat_bit;
>>> + uint32_t src;
>>> + bool masked;
>>> +
>>> + if (!pnv_psi_irq_bits(psi, irq, &xivr_reg, &stat_reg, &stat_bit)) {
>>> + qemu_log_mask(LOG_GUEST_ERROR, "PSI: Unsupported irq %d\n", irq);
>>> + return;
>>> + }
>>> +
>>> + src = (psi->regs[xivr_reg] & PSIHB_XIVR_SRC_MSK) >> PSIHB_XIVR_SRC_SH;
>>> + masked = (psi->regs[xivr_reg] & PSIHB_XIVR_PRIO_MSK) ==
>>> PSIHB_XIVR_PRIO_MSK;
>>> + if (state) {
>>> + psi->regs[stat_reg] |= stat_bit;
>>> + /* XXX optimization: check mask here. That means re-evaluating
>>> + * when unmasking, thus TODO
>>> + */
>>> + qemu_irq_raise(ics->qirqs[src]);
>>> + } else {
>>> + psi->regs[stat_reg] &= ~stat_bit;
>>> +
>>> + /* FSP and PSI are muxed so don't lower if either still set */
>>> + if (stat_reg != PSIHB_XSCOM_CR ||
>>> + !(psi->regs[stat_reg] & (PSIHB_CR_PSI_IRQ |
>>> PSIHB_CR_FSP_IRQ))) {
>>> + qemu_irq_lower(ics->qirqs[src]);
>>> + } else {
>>> + state = true;
ugly.
>>> + }
>>> + }
>>
>> It might be cleaner to just revaluate the irq level from scratch here,
>> and set the level, rather than doing this complicated dance to work
>> out if it has changed.
>
> OK. I need to take a closer look at that.
So I took a closer a look and some parts are not clear, even if
correct. The FSP and PSI interrupts are muxed and the above code
tries to re-conciliate the IRQ triggering in a single routine.
But we ended up (I think) using some hacks. See below PnvPsiIrq.
[ ... ]
>>> +static void pnv_psi_realize(DeviceState *dev, Error **errp)
>>> +{
>>> + PnvPsi *psi = PNV_PSI(dev);
>>> + ICSState *ics = &psi->ics;
>>> + Object *obj;
>>> + Error *err = NULL, *local_err = NULL;
>>> + unsigned int i;
>>> +
>>> + /* Initialize MMIO region */
>>> + memory_region_init_io(&psi->regs_mr, OBJECT(dev), &psi_mmio_ops, psi,
>>> + "psihb", PNV_PSIHB_BAR_SIZE);
>>> +
>>> + /* Default BAR. Use object properties ? */
>>> + pnv_psi_set_bar(psi, PNV_PSIHB_BAR | PSIHB_BAR_EN);
>>> +
>>> + /* Default sources in XIVR */
>>> + psi->regs[PSIHB_XSCOM_XIVR_PSI] = PSIHB_XIVR_PRIO_MSK |
>>> + (0ull << PSIHB_XIVR_SRC_SH);
>>> + psi->regs[PSIHB_XSCOM_XIVR_OCC] = PSIHB_XIVR_PRIO_MSK |
>>> + (1ull << PSIHB_XIVR_SRC_SH);
>>> + psi->regs[PSIHB_XSCOM_XIVR_FSI] = PSIHB_XIVR_PRIO_MSK |
>>> + (2ull << PSIHB_XIVR_SRC_SH);
>>> + psi->regs[PSIHB_XSCOM_XIVR_LPCI2C] = PSIHB_XIVR_PRIO_MSK |
>>> + (3ull << PSIHB_XIVR_SRC_SH);
>>> + psi->regs[PSIHB_XSCOM_XIVR_LOCERR] = PSIHB_XIVR_PRIO_MSK |
>>> + (4ull << PSIHB_XIVR_SRC_SH);
>>> + psi->regs[PSIHB_XSCOM_XIVR_EXT] = PSIHB_XIVR_PRIO_MSK |
>>> + (5ull << PSIHB_XIVR_SRC_SH);
>>> +
The above is not using a loop on PnvPsiIrq because the numbers
do not match the PSI IRQ definitions.
[ ... ]
>>> +
>>> +typedef enum PnvPsiIrq {
>>> + PSIHB_IRQ_PSI, /* internal use only */
>>> + PSIHB_IRQ_FSP, /* internal use only */
>>> + PSIHB_IRQ_OCC,
>>> + PSIHB_IRQ_FSI,
>>> + PSIHB_IRQ_LPC_I2C,
>>> + PSIHB_IRQ_LOCAL_ERR,
>>> + PSIHB_IRQ_EXTERNAL,
>>> +} PnvPsiIrq;
>>> +
>>> +#define PSI_NUM_INTERRUPTS 6
a maximum of 6 interrupts for an enum containing 7 entries. It is a
little ugly.
I am going to rewrite this part in a more straight forward way.
C.
