* [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* [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper 2020-11-29 17:40 [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself Peter Maydell @ 2020-11-29 17:40 ` Peter Maydell 2020-11-30 5:41 ` Wu, Wentong 2020-11-30 12:13 ` Philippe Mathieu-Daudé 2020-11-29 17:40 ` [PATCH v2 2/3] target/nios2: Move nios2_check_interrupts() into target/nios2 Peter Maydell ` (2 subsequent siblings) 3 siblings, 2 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. Create named "IRQ" GPIO inputs to the Nios2 CPU object, and make the only user of the IIC wire up directly to those instead. Note that the old code had an "NMI" concept which was entirely unused and also as far as I can see not architecturally correct, since only the EIC has a concept of an NMI. This fixes a Coverity-reported trivial memory leak of the IRQ array allocated in nios2_cpu_pic_init(). Fixes: Coverity CID 1421916 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/nios2/cpu.h | 1 - hw/intc/nios2_iic.c | 95 --------------------------------------- hw/nios2/10m50_devboard.c | 13 +----- hw/nios2/cpu_pic.c | 31 ------------- target/nios2/cpu.c | 30 +++++++++++++ MAINTAINERS | 1 - hw/intc/meson.build | 1 - 7 files changed, 32 insertions(+), 140 deletions(-) delete mode 100644 hw/intc/nios2_iic.c diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h index 86bbe1d8670..b7efb54ba7e 100644 --- a/target/nios2/cpu.h +++ b/target/nios2/cpu.h @@ -201,7 +201,6 @@ void nios2_cpu_do_unaligned_access(CPUState *cpu, vaddr addr, MMUAccessType access_type, int mmu_idx, uintptr_t retaddr); -qemu_irq *nios2_cpu_pic_init(Nios2CPU *cpu); void nios2_check_interrupts(CPUNios2State *env); void do_nios2_semihosting(CPUNios2State *env); diff --git a/hw/intc/nios2_iic.c b/hw/intc/nios2_iic.c deleted file mode 100644 index 216db670594..00000000000 --- a/hw/intc/nios2_iic.c +++ /dev/null @@ -1,95 +0,0 @@ -/* - * QEMU Altera Internal Interrupt Controller. - * - * Copyright (c) 2012 Chris Wulff <crwulff@gmail.com> - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, see - * <http://www.gnu.org/licenses/lgpl-2.1.html> - */ - -#include "qemu/osdep.h" -#include "qemu/module.h" -#include "qapi/error.h" - -#include "hw/irq.h" -#include "hw/sysbus.h" -#include "cpu.h" -#include "qom/object.h" - -#define TYPE_ALTERA_IIC "altera,iic" -OBJECT_DECLARE_SIMPLE_TYPE(AlteraIIC, ALTERA_IIC) - -struct AlteraIIC { - SysBusDevice parent_obj; - void *cpu; - qemu_irq parent_irq; -}; - -static void update_irq(AlteraIIC *pv) -{ - CPUNios2State *env = &((Nios2CPU *)(pv->cpu))->env; - - qemu_set_irq(pv->parent_irq, - env->regs[CR_IPENDING] & env->regs[CR_IENABLE]); -} - -static void irq_handler(void *opaque, int irq, int level) -{ - AlteraIIC *pv = opaque; - CPUNios2State *env = &((Nios2CPU *)(pv->cpu))->env; - - env->regs[CR_IPENDING] &= ~(1 << irq); - env->regs[CR_IPENDING] |= !!level << irq; - - update_irq(pv); -} - -static void altera_iic_init(Object *obj) -{ - AlteraIIC *pv = ALTERA_IIC(obj); - - qdev_init_gpio_in(DEVICE(pv), irq_handler, 32); - sysbus_init_irq(SYS_BUS_DEVICE(obj), &pv->parent_irq); -} - -static void altera_iic_realize(DeviceState *dev, Error **errp) -{ - struct AlteraIIC *pv = ALTERA_IIC(dev); - - pv->cpu = object_property_get_link(OBJECT(dev), "cpu", &error_abort); -} - -static void altera_iic_class_init(ObjectClass *klass, void *data) -{ - DeviceClass *dc = DEVICE_CLASS(klass); - - /* Reason: needs to be wired up, e.g. by nios2_10m50_ghrd_init() */ - dc->user_creatable = false; - dc->realize = altera_iic_realize; -} - -static TypeInfo altera_iic_info = { - .name = TYPE_ALTERA_IIC, - .parent = TYPE_SYS_BUS_DEVICE, - .instance_size = sizeof(AlteraIIC), - .instance_init = altera_iic_init, - .class_init = altera_iic_class_init, -}; - -static void altera_iic_register(void) -{ - type_register_static(&altera_iic_info); -} - -type_init(altera_iic_register) diff --git a/hw/nios2/10m50_devboard.c b/hw/nios2/10m50_devboard.c index 5c13b74306f..a14fc31e86b 100644 --- a/hw/nios2/10m50_devboard.c +++ b/hw/nios2/10m50_devboard.c @@ -52,7 +52,7 @@ static void nios2_10m50_ghrd_init(MachineState *machine) ram_addr_t tcm_size = 0x1000; /* 1 kiB, but QEMU limit is 4 kiB */ ram_addr_t ram_base = 0x08000000; ram_addr_t ram_size = 0x08000000; - qemu_irq *cpu_irq, irq[32]; + qemu_irq irq[32]; int i; /* Physical TCM (tb_ram_1k) with alias at 0xc0000000 */ @@ -75,17 +75,8 @@ static void nios2_10m50_ghrd_init(MachineState *machine) /* Create CPU -- FIXME */ cpu = NIOS2_CPU(cpu_create(TYPE_NIOS2_CPU)); - - /* Register: CPU interrupt controller (PIC) */ - cpu_irq = nios2_cpu_pic_init(cpu); - - /* Register: Internal Interrupt Controller (IIC) */ - dev = qdev_new("altera,iic"); - object_property_add_const_link(OBJECT(dev), "cpu", OBJECT(cpu)); - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irq[0]); for (i = 0; i < 32; i++) { - irq[i] = qdev_get_gpio_in(dev, i); + irq[i] = qdev_get_gpio_in_named(DEVICE(cpu), "IRQ", i); } /* Register: Altera 16550 UART */ diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c index 5ea7e52ab83..3fb621c5c85 100644 --- a/hw/nios2/cpu_pic.c +++ b/hw/nios2/cpu_pic.c @@ -26,32 +26,6 @@ #include "boot.h" -static void nios2_pic_cpu_handler(void *opaque, int irq, int level) -{ - Nios2CPU *cpu = opaque; - CPUNios2State *env = &cpu->env; - CPUState *cs = CPU(cpu); - int type = irq ? CPU_INTERRUPT_NMI : CPU_INTERRUPT_HARD; - - if (type == CPU_INTERRUPT_HARD) { - env->irq_pending = level; - - if (level && (env->regs[CR_STATUS] & CR_STATUS_PIE)) { - env->irq_pending = 0; - cpu_interrupt(cs, type); - } else if (!level) { - env->irq_pending = 0; - cpu_reset_interrupt(cs, type); - } - } else { - if (level) { - cpu_interrupt(cs, type); - } else { - cpu_reset_interrupt(cs, type); - } - } -} - void nios2_check_interrupts(CPUNios2State *env) { if (env->irq_pending && @@ -60,8 +34,3 @@ void nios2_check_interrupts(CPUNios2State *env) cpu_interrupt(env_cpu(env), CPU_INTERRUPT_HARD); } } - -qemu_irq *nios2_cpu_pic_init(Nios2CPU *cpu) -{ - return qemu_allocate_irqs(nios2_pic_cpu_handler, cpu, 2); -} diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c index 8f7011fcb92..52ebda89ca7 100644 --- a/target/nios2/cpu.c +++ b/target/nios2/cpu.c @@ -64,6 +64,27 @@ static void nios2_cpu_reset(DeviceState *dev) #endif } +#ifndef CONFIG_USER_ONLY +static void nios2_cpu_set_irq(void *opaque, int irq, int level) +{ + Nios2CPU *cpu = opaque; + CPUNios2State *env = &cpu->env; + CPUState *cs = CPU(cpu); + + env->regs[CR_IPENDING] &= ~(1 << irq); + env->regs[CR_IPENDING] |= !!level << irq; + + env->irq_pending = env->regs[CR_IPENDING] & env->regs[CR_IENABLE]; + + if (env->irq_pending && (env->regs[CR_STATUS] & CR_STATUS_PIE)) { + env->irq_pending = 0; + cpu_interrupt(cs, CPU_INTERRUPT_HARD); + } else if (!env->irq_pending) { + cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD); + } +} +#endif + static void nios2_cpu_initfn(Object *obj) { Nios2CPU *cpu = NIOS2_CPU(obj); @@ -72,6 +93,15 @@ static void nios2_cpu_initfn(Object *obj) #if !defined(CONFIG_USER_ONLY) mmu_init(&cpu->env); + + /* + * These interrupt lines model the IIC (internal interrupt + * controller). QEMU does not currently support the EIC + * (external interrupt controller) -- if we did it would be + * a separate device in hw/intc with a custom interface to + * the CPU, and boards using it would not wire up these IRQ lines. + */ + qdev_init_gpio_in_named(DEVICE(cpu), nios2_cpu_set_irq, "IRQ", 32); #endif } diff --git a/MAINTAINERS b/MAINTAINERS index 68bc160f41b..1bf7d02330e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -259,7 +259,6 @@ M: Marek Vasut <marex@denx.de> S: Maintained F: target/nios2/ F: hw/nios2/ -F: hw/intc/nios2_iic.c F: disas/nios2.c F: default-configs/nios2-softmmu.mak diff --git a/hw/intc/meson.build b/hw/intc/meson.build index 3f82cc230ad..7c3e9daf586 100644 --- a/hw/intc/meson.build +++ b/hw/intc/meson.build @@ -37,7 +37,6 @@ specific_ss.add(when: 'CONFIG_IBEX', if_true: files('ibex_plic.c')) specific_ss.add(when: 'CONFIG_IOAPIC', if_true: files('ioapic.c')) specific_ss.add(when: 'CONFIG_LOONGSON_LIOINTC', if_true: files('loongson_liointc.c')) specific_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('mips_gic.c')) -specific_ss.add(when: 'CONFIG_NIOS2', if_true: files('nios2_iic.c')) specific_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_intc.c')) specific_ss.add(when: 'CONFIG_OMPIC', if_true: files('ompic.c')) specific_ss.add(when: 'CONFIG_OPENPIC_KVM', if_true: files('openpic_kvm.c')) -- 2.20.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper 2020-11-29 17:40 ` [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper Peter Maydell @ 2020-11-30 5:41 ` Wu, Wentong 2020-11-30 9:53 ` Peter Maydell 2020-11-30 12:13 ` Philippe Mathieu-Daudé 1 sibling, 1 reply; 11+ messages in thread From: Wu, Wentong @ 2020-11-30 5:41 UTC (permalink / raw) To: Peter Maydell, qemu-devel; +Cc: Marek Vasut, Sandra Loosemore, Chris Wulff On Monday, November 30, 2020 1:40 AM, Peter Maydell wrote: > 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. > > Create named "IRQ" GPIO inputs to the Nios2 CPU object, and make the only > user of the IIC wire up directly to those instead. > > Note that the old code had an "NMI" concept which was entirely unused and > also as far as I can see not architecturally correct, since only the EIC has a > concept of an NMI. > > This fixes a Coverity-reported trivial memory leak of the IRQ array allocated in > nios2_cpu_pic_init(). > > Fixes: Coverity CID 1421916 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/nios2/cpu.h | 1 - > hw/intc/nios2_iic.c | 95 --------------------------------------- > hw/nios2/10m50_devboard.c | 13 +----- > hw/nios2/cpu_pic.c | 31 ------------- > target/nios2/cpu.c | 30 +++++++++++++ > MAINTAINERS | 1 - > hw/intc/meson.build | 1 - > 7 files changed, 32 insertions(+), 140 deletions(-) delete mode 100644 > hw/intc/nios2_iic.c Reviewed and tested. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper 2020-11-30 5:41 ` Wu, Wentong @ 2020-11-30 9:53 ` Peter Maydell 2020-11-30 13:12 ` Wu, Wentong 0 siblings, 1 reply; 11+ messages in thread From: Peter Maydell @ 2020-11-30 9:53 UTC (permalink / raw) To: Wu, Wentong; +Cc: Marek Vasut, Sandra Loosemore, Chris Wulff, qemu-devel On Mon, 30 Nov 2020 at 05:41, Wu, Wentong <wentong.wu@intel.com> wrote: > Reviewed and tested. Thanks! Can I put that in as Reviewed-by/Tested-by lines? -- PMM ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper 2020-11-30 9:53 ` Peter Maydell @ 2020-11-30 13:12 ` Wu, Wentong 0 siblings, 0 replies; 11+ messages in thread From: Wu, Wentong @ 2020-11-30 13:12 UTC (permalink / raw) To: Peter Maydell; +Cc: Marek Vasut, Sandra Loosemore, Chris Wulff, qemu-devel On Monday, November 30, 2020 5:54 PM, Peter Maydell wrote: > On Mon, 30 Nov 2020 at 05:41, Wu, Wentong <wentong.wu@intel.com> wrote: > > Reviewed and tested. > > Thanks! Can I put that in as Reviewed-by/Tested-by lines? Sure and my pleasure, thanks Peter! > > -- PMM ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper 2020-11-29 17:40 ` [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper Peter Maydell 2020-11-30 5:41 ` Wu, Wentong @ 2020-11-30 12:13 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 11+ messages in thread From: Philippe Mathieu-Daudé @ 2020-11-30 12:13 UTC (permalink / raw) To: Peter Maydell, qemu-devel Cc: Marek Vasut, Sandra Loosemore, Chris Wulff, Wentong Wu On 11/29/20 6:40 PM, Peter Maydell wrote: > 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. > > Create named "IRQ" GPIO inputs to the Nios2 CPU object, and make the > only user of the IIC wire up directly to those instead. > > Note that the old code had an "NMI" concept which was entirely unused > and also as far as I can see not architecturally correct, since only > the EIC has a concept of an NMI. > > This fixes a Coverity-reported trivial memory leak of the IRQ array > allocated in nios2_cpu_pic_init(). > > Fixes: Coverity CID 1421916 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/nios2/cpu.h | 1 - > hw/intc/nios2_iic.c | 95 --------------------------------------- > hw/nios2/10m50_devboard.c | 13 +----- > hw/nios2/cpu_pic.c | 31 ------------- > target/nios2/cpu.c | 30 +++++++++++++ > MAINTAINERS | 1 - > hw/intc/meson.build | 1 - > 7 files changed, 32 insertions(+), 140 deletions(-) > delete mode 100644 hw/intc/nios2_iic.c Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] target/nios2: Move nios2_check_interrupts() into target/nios2 2020-11-29 17:40 [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself Peter Maydell 2020-11-29 17:40 ` [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper Peter Maydell @ 2020-11-29 17:40 ` Peter Maydell 2020-11-30 5:43 ` Wu, Wentong 2020-11-29 17:40 ` [PATCH v2 3/3] target/nios2: Use deposit32() to update ipending register Peter Maydell 2020-12-11 14:05 ` [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself Peter Maydell 3 siblings, 1 reply; 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 function nios2_check_interrupts)() looks only at CPU-internal state; it belongs in target/nios2, not hw/nios2. Move it into the same file as its only caller, so it can just be local to that file. This removes the only remaining code from cpu_pic.c, so we can delete that file entirely. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- target/nios2/cpu.h | 2 -- hw/nios2/cpu_pic.c | 36 ------------------------------------ target/nios2/op_helper.c | 9 +++++++++ hw/nios2/meson.build | 2 +- 4 files changed, 10 insertions(+), 39 deletions(-) delete mode 100644 hw/nios2/cpu_pic.c diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h index b7efb54ba7e..2ab82fdc713 100644 --- a/target/nios2/cpu.h +++ b/target/nios2/cpu.h @@ -201,8 +201,6 @@ void nios2_cpu_do_unaligned_access(CPUState *cpu, vaddr addr, MMUAccessType access_type, int mmu_idx, uintptr_t retaddr); -void nios2_check_interrupts(CPUNios2State *env); - void do_nios2_semihosting(CPUNios2State *env); #define CPU_RESOLVING_TYPE TYPE_NIOS2_CPU diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c deleted file mode 100644 index 3fb621c5c85..00000000000 --- a/hw/nios2/cpu_pic.c +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Altera Nios2 CPU PIC - * - * Copyright (c) 2016 Marek Vasut <marek.vasut@gmail.com> - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, see - * <http://www.gnu.org/licenses/lgpl-2.1.html> - */ - -#include "qemu/osdep.h" -#include "cpu.h" -#include "hw/irq.h" - -#include "qemu/config-file.h" - -#include "boot.h" - -void nios2_check_interrupts(CPUNios2State *env) -{ - if (env->irq_pending && - (env->regs[CR_STATUS] & CR_STATUS_PIE)) { - env->irq_pending = 0; - cpu_interrupt(env_cpu(env), CPU_INTERRUPT_HARD); - } -} diff --git a/target/nios2/op_helper.c b/target/nios2/op_helper.c index a60730faac3..a59003855ab 100644 --- a/target/nios2/op_helper.c +++ b/target/nios2/op_helper.c @@ -36,6 +36,15 @@ void helper_mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v) mmu_write(env, rn, v); } +static void nios2_check_interrupts(CPUNios2State *env) +{ + if (env->irq_pending && + (env->regs[CR_STATUS] & CR_STATUS_PIE)) { + env->irq_pending = 0; + cpu_interrupt(env_cpu(env), CPU_INTERRUPT_HARD); + } +} + void helper_check_interrupts(CPUNios2State *env) { qemu_mutex_lock_iothread(); diff --git a/hw/nios2/meson.build b/hw/nios2/meson.build index dd66ebb32f6..6c58e8082b4 100644 --- a/hw/nios2/meson.build +++ b/hw/nios2/meson.build @@ -1,5 +1,5 @@ nios2_ss = ss.source_set() -nios2_ss.add(files('boot.c', 'cpu_pic.c')) +nios2_ss.add(files('boot.c')) nios2_ss.add(when: 'CONFIG_NIOS2_10M50', if_true: files('10m50_devboard.c')) nios2_ss.add(when: 'CONFIG_NIOS2_GENERIC_NOMMU', if_true: files('generic_nommu.c')) -- 2.20.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH v2 2/3] target/nios2: Move nios2_check_interrupts() into target/nios2 2020-11-29 17:40 ` [PATCH v2 2/3] target/nios2: Move nios2_check_interrupts() into target/nios2 Peter Maydell @ 2020-11-30 5:43 ` Wu, Wentong 0 siblings, 0 replies; 11+ messages in thread From: Wu, Wentong @ 2020-11-30 5:43 UTC (permalink / raw) To: Peter Maydell, qemu-devel; +Cc: Marek Vasut, Sandra Loosemore, Chris Wulff On Monday, November 30, 2020 1:40 AM, Peter Maydell wrote: > The function nios2_check_interrupts)() looks only at CPU-internal state; it > belongs in target/nios2, not hw/nios2. Move it into the same file as its only > caller, so it can just be local to that file. > > This removes the only remaining code from cpu_pic.c, so we can delete that file > entirely. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/nios2/cpu.h | 2 -- > hw/nios2/cpu_pic.c | 36 ------------------------------------ > target/nios2/op_helper.c | 9 +++++++++ > hw/nios2/meson.build | 2 +- > 4 files changed, 10 insertions(+), 39 deletions(-) delete mode 100644 > hw/nios2/cpu_pic.c Reviewed and tested. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] target/nios2: Use deposit32() to update ipending register 2020-11-29 17:40 [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself Peter Maydell 2020-11-29 17:40 ` [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper Peter Maydell 2020-11-29 17:40 ` [PATCH v2 2/3] target/nios2: Move nios2_check_interrupts() into target/nios2 Peter Maydell @ 2020-11-29 17:40 ` Peter Maydell 2020-11-30 9:55 ` Philippe Mathieu-Daudé 2020-12-11 14:05 ` [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself Peter Maydell 3 siblings, 1 reply; 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 In nios2_cpu_set_irq(), use deposit32() rather than raw shift-and-mask operations to set the appropriate bit in the ipending register. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- In patch 1 I left the code for this identical to the old code from nios2_iic.c for clarity of that refactoring, but deposit32() is a clearer way to write it. --- target/nios2/cpu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c index 52ebda89ca7..58688e1623a 100644 --- a/target/nios2/cpu.c +++ b/target/nios2/cpu.c @@ -71,8 +71,7 @@ static void nios2_cpu_set_irq(void *opaque, int irq, int level) CPUNios2State *env = &cpu->env; CPUState *cs = CPU(cpu); - env->regs[CR_IPENDING] &= ~(1 << irq); - env->regs[CR_IPENDING] |= !!level << irq; + env->regs[CR_IPENDING] = deposit32(env->regs[CR_IPENDING], irq, 1, !!level); env->irq_pending = env->regs[CR_IPENDING] & env->regs[CR_IENABLE]; -- 2.20.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] target/nios2: Use deposit32() to update ipending register 2020-11-29 17:40 ` [PATCH v2 3/3] target/nios2: Use deposit32() to update ipending register Peter Maydell @ 2020-11-30 9:55 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 11+ messages in thread From: Philippe Mathieu-Daudé @ 2020-11-30 9:55 UTC (permalink / raw) To: Peter Maydell, qemu-devel Cc: Marek Vasut, Sandra Loosemore, Chris Wulff, Wentong Wu On 11/29/20 6:40 PM, Peter Maydell wrote: > In nios2_cpu_set_irq(), use deposit32() rather than raw shift-and-mask > operations to set the appropriate bit in the ipending register. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > In patch 1 I left the code for this identical to the old > code from nios2_iic.c for clarity of that refactoring, > but deposit32() is a clearer way to write it. > --- > target/nios2/cpu.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself 2020-11-29 17:40 [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself Peter Maydell ` (2 preceding siblings ...) 2020-11-29 17:40 ` [PATCH v2 3/3] target/nios2: Use deposit32() to update ipending register Peter Maydell @ 2020-12-11 14:05 ` Peter Maydell 3 siblings, 0 replies; 11+ messages in thread From: Peter Maydell @ 2020-12-11 14:05 UTC (permalink / raw) To: QEMU Developers; +Cc: Marek Vasut, Sandra Loosemore, Chris Wulff, Wentong Wu On Sun, 29 Nov 2020 at 17:40, Peter Maydell <peter.maydell@linaro.org> wrote: > > 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 :-) Now the tree is open for 6.0 development I'll take this series via target-arm.next. thanks -- PMM ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-12-11 15:24 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-29 17:40 [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself Peter Maydell 2020-11-29 17:40 ` [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper Peter Maydell 2020-11-30 5:41 ` Wu, Wentong 2020-11-30 9:53 ` Peter Maydell 2020-11-30 13:12 ` Wu, Wentong 2020-11-30 12:13 ` Philippe Mathieu-Daudé 2020-11-29 17:40 ` [PATCH v2 2/3] target/nios2: Move nios2_check_interrupts() into target/nios2 Peter Maydell 2020-11-30 5:43 ` Wu, Wentong 2020-11-29 17:40 ` [PATCH v2 3/3] target/nios2: Use deposit32() to update ipending register Peter Maydell 2020-11-30 9:55 ` Philippe Mathieu-Daudé 2020-12-11 14:05 ` [PATCH v2 0/3] target/nios2: Roll cpu_pic/nios2_iic code into CPU itself Peter Maydell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.