[PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself

* [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself
@ 2020-11-29 17:40 Peter Maydell
  2020-11-29 17:40 ` [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper Peter Maydell
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Peter Maydell @ 2020-11-29 17:40 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marek Vasut, Sandra Loosemore, Chris Wulff, Wentong Wu

The Nios2 architecture supports two different interrupt controller
options:

 * The IIC (Internal Interrupt Controller) is part of the CPU itself;
   it has 32 IRQ input lines and no NMI support.  Interrupt status is
   queried and controlled via the CPU's ipending and istatus
   registers.

 * The EIC (External Interrupt Controller) interface allows the CPU
   to connect to an external interrupt controller.  The interface
   allows the interrupt controller to present a packet of information
   containing:
    - handler address
    - interrupt level
    - register set
    - NMI mode

QEMU does not model an EIC currently.  We do model the IIC, but its
implementation is split across code in hw/nios2/cpu_pic.c and
hw/intc/nios2_iic.c.  The code in those two files has no state of its
own -- the IIC state is in the Nios2CPU state struct.

Because CPU objects now inherit (indirectly) from TYPE_DEVICE, they
can have GPIO input lines themselves, so we can implement the IIC
directly in the CPU object the same way that real hardware does.

This fixes a Coverity-reported trivial memory leak of the IRQ array
allocated in nios2_cpu_pic_init().  I think the diffstat on the
overall patchset is also a pretty good argument for the refactor :-)

If we did ever want to model an EIC we'd do it like this:
 * define a TYPE_EIC_INTERFACE QOM interface corresponding to the
   hardware's EIC interface.  This would probably be something like
   just a single method function (to be implemented by the CPU) with
   a signature
    request_interrupt(uint32_t handler_address,
                      uint8_t register_set,
                      uint8_t irq_level,
                      bool is_nmi)
 * implement that interface on the CPU to have the required behaviour
   (take the interrupt if irq_level allows, etc, etc)
 * add a QOM property to the CPU for "disable the IIC" (I think the
   only needed behaviour change for IIC disabled would be to make
   "ipending" and "ienable" RAZ/WI)
 * implement the EIC as an external device in hw/intc/ with whatever
   internal state, guest-visible registers, etc the specific EIC
   implementation defines. If the EIC allows daisy-chaining, it
   should implement TYPE_EIC_INTERFACE itself as well.
 * the EIC object defines a QOM link property that accepts links
   to objects defining TYPE_EIC_INTERFACE
 * board models using the EIC should:
    - set the "disable the IIC" property on the CPU
    - create the EIC
    - pass the CPU to the EIC's TYPE_EIC_INTERFACE link property

Changes v1->v2:
 * patch 1 now rolls the hw/intc/nios2_iic.c code into the CPU too
 * patch 3 is new: a trivial change to some code that I moved
   without changing in patch 1 to use deposit32()

thanks
-- PMM

Peter Maydell (3):
  target/nios2: Move IIC code into CPU object proper
  target/nios2: Move nios2_check_interrupts() into target/nios2
  target/nios2: Use deposit32() to update ipending register

 target/nios2/cpu.h        |  3 --
 hw/intc/nios2_iic.c       | 95 ---------------------------------------
 hw/nios2/10m50_devboard.c | 13 +-----
 hw/nios2/cpu_pic.c        | 67 ---------------------------
 target/nios2/cpu.c        | 29 ++++++++++++
 target/nios2/op_helper.c  |  9 ++++
 MAINTAINERS               |  1 -
 hw/intc/meson.build       |  1 -
 hw/nios2/meson.build      |  2 +-
 9 files changed, 41 insertions(+), 179 deletions(-)
 delete mode 100644 hw/intc/nios2_iic.c
 delete mode 100644 hw/nios2/cpu_pic.c

-- 
2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread