Re: [PATCH 1/2] target/nios2: Move cpu_pic code into CPU object proper

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

On Sat, 28 Nov 2020 at 05:50, Wu, Wentong <wentong.wu@intel.com> wrote:
> The code looks ok to me, and I tested the changes on Zephyr project, it works well.
>
> But, according https://www.intel.com/content/dam/www/programmable/us/en/pdfs/literature/hb/nios2/n2sw_nii52006.pdf ,
> The Nios II processor offers two distinct approaches to handling hardware interrupts:
> ■ The internal interrupt controller (IIC)
> ■ The external interrupt controller (EIC) interface
>
> We have already defined TypeInfo named "altera,iic" , and others can also define EIC, so IMHO I don't think we should replace the internal interrupt controller with GPIO.

The "altera,iic" device is what connects to these GPIO lines -- the
single output line from the "altera,iic" device connects to the
"IRQ" GPIO input. The code currently in cpu_pic.c is in no way
an external-to-the-CPU device implementation:
 * it's not a device
 * it directly messes with CPUNios2State fields like irq_pending
   and env->regs[CR_STATUS]
It's been implemented as part of the CPU, it's just in the
wrong place in QEMU's source code and not very cleanly
connected to the rest of the system.

If we ever wanted to model an EIC, we'd need to also model the
EIC-to-CPU interface, which seems to be moderately complicated
(the EIC "presents one interrupt to the Nios II processor, with
interrupt handler address and register set selection information").
So we'd do that by modelling a suitable interface connection plus
the EIC device, and an board model with an EIC would wire that up
and simply not connect the NMI/IRQ GPIO lines to anything, which
would be the equivalent of "the IIC is disabled" (or if just not
connecting the inputs is insufficient, we'd have a QOM property
on the CPU object for "disable the IIC", which would be an exact
match for "to 'configure the h/w with the IIC not implemented").

You have, though, prompted me to look at hw/intc/nios2_iic.c,
which I had previously assumed was a real external interrupt controller,
and although that is coded as a separate device, it has no
internal state of its own -- it also is just looking directly
at the CPUNios2State fields and register state. It's part of the
CPU, it's just implemented in the wrong place in QEMU.

So I'll spin a v2 of this series that also folds that code properly
into the CPU object, so that the CPU object provides 32 input IRQ
lines. That will mean we're modelling the hardware much more closely:
 * the IIC is internal to the CPU itself
 * (hypothetical) board models using an EIC will provide an
   EIC model that connects to the CPU using an interface similar
   to what the real h/w does
 * if necessary, QOM property for the equivalent of "configure
   CPU with the IIC not implemented"

thanks
-- PMM