| Message ID | 20191018154212.13458-9-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Clean-ups: remove QDEV_PROP_PTR | expand |
On Fri, 18 Oct 2019 at 16:43, Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > Instead of using raw interrupt vector pointer, store the associated > CPU with a link property. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> A link property is reasonable for a tightly coupled CPU and interrupt-controller. But in this case the binding is not actually very tight, and we can avoid it. > @@ -298,7 +296,7 @@ void axisdev88_init(MachineState *machine) > > dev = qdev_create(NULL, "etraxfs,pic"); > /* FIXME: Is there a proper way to signal vectors to the CPU core? */ > - qdev_prop_set_ptr(dev, "interrupt_vector", &env->interrupt_vector); > + object_property_set_link(OBJECT(dev), OBJECT(cpu), "cpu", &error_abort); Rather than using a link property like this, we could instead make use of the fact that a qemu_irq line is actually capable of passing an arbitrary "int" value, not merely a bool. To do this we would: * remove the FIXME comment and the ptr prop/link prop code here * remove the definition of the property from the PIC device > @@ -79,9 +78,10 @@ static void pic_update(struct etrax_pic *fs) > } > } > > - if (fs->interrupt_vector) { > - /* hack alert: ptr property */ > - *(uint32_t*)(fs->interrupt_vector) = vector; > + if (fs->cpu) { > + /* hack alert: cpu link property */ > + int32_t *int_vec = &fs->cpu->env.interrupt_vector; > + *int_vec = (uint32_t)vector; > } > qemu_set_irq(fs->parent_irq, !!vector); * here, instead of setting the CPU interrupt_vector field and passing !!vector to qemu_set_irq, we just pass "vector", so the other end gets the whole integer * in target/cris/cpu.c:cris_cpu_set_irq(), we add something like if (irq == CRIS_CPU_IRQ) { /* * The PIC passes us the vector for the IRQ as the value it sends * over the qemu_irq line */ cpu->interrupt_vector = value; } at the top of the function. It would also be nice somewhere to have a comment documenting that this is the semantics the CPU expects for its incoming IRQ line. Unless anybody has a better place, then perhaps in the part of target/cris/cpu.h that defines CRIS_CPU_IRQ. (If the PIC followed the just-recently-invented qdev convention of having a .h file with a comment defining the "QEMU interface" to the device, as eg include/hw/misc/armsse-mhu.h, then that comment would be a good place to note that its sysbus IRQ 0 has these value-is-the-vector semantics. But it doesn't.) > } > @@ -164,7 +164,7 @@ static void etraxfs_pic_init(Object *obj) > } > > static Property etraxfs_pic_properties[] = { > - DEFINE_PROP_PTR("interrupt_vector", struct etrax_pic, interrupt_vector), > + DEFINE_PROP_LINK("cpu", struct etrax_pic, cpu, TYPE_CRIS_CPU, CRISCPU *), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -173,10 +173,6 @@ static void etraxfs_pic_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->props = etraxfs_pic_properties; > - /* > - * Note: pointer property "interrupt_vector" may remain null, thus > - * no need for dc->user_creatable = false; > - */ > } Incidentally this is a sysbus device, so it's not user creatable anyway. thanks -- PMM
diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c index 940c7dd122..cb7b1b58aa 100644 --- a/hw/cris/axis_dev88.c +++ b/hw/cris/axis_dev88.c @@ -253,7 +253,6 @@ void axisdev88_init(MachineState *machine) const char *kernel_filename = machine->kernel_filename; const char *kernel_cmdline = machine->kernel_cmdline; CRISCPU *cpu; - CPUCRISState *env; DeviceState *dev; SysBusDevice *s; DriveInfo *nand; @@ -267,7 +266,6 @@ void axisdev88_init(MachineState *machine) /* init CPUs */ cpu = CRIS_CPU(cpu_create(machine->cpu_type)); - env = &cpu->env; /* allocate RAM */ memory_region_allocate_system_memory(phys_ram, NULL, "axisdev88.ram", @@ -298,7 +296,7 @@ void axisdev88_init(MachineState *machine) dev = qdev_create(NULL, "etraxfs,pic"); /* FIXME: Is there a proper way to signal vectors to the CPU core? */ - qdev_prop_set_ptr(dev, "interrupt_vector", &env->interrupt_vector); + object_property_set_link(OBJECT(dev), OBJECT(cpu), "cpu", &error_abort); qdev_init_nofail(dev); s = SYS_BUS_DEVICE(dev); sysbus_mmio_map(s, 0, 0x3001c000); diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs index f726d87532..26a3cb36cb 100644 --- a/hw/intc/Makefile.objs +++ b/hw/intc/Makefile.objs @@ -5,7 +5,6 @@ common-obj-$(CONFIG_PUV3) += puv3_intc.o common-obj-$(CONFIG_XILINX) += xilinx_intc.o common-obj-$(CONFIG_XLNX_ZYNQMP_PMU) += xlnx-pmu-iomod-intc.o common-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-ipi.o -common-obj-$(CONFIG_ETRAXFS) += etraxfs_pic.o common-obj-$(CONFIG_IMX) += imx_avic.o imx_gpcv2.o common-obj-$(CONFIG_LM32) += lm32_pic.o common-obj-$(CONFIG_REALVIEW) += realview_gic.o @@ -49,3 +48,4 @@ obj-$(CONFIG_ARM_GIC) += arm_gicv3_cpuif.o obj-$(CONFIG_MIPS_CPS) += mips_gic.o obj-$(CONFIG_NIOS2) += nios2_iic.o obj-$(CONFIG_OMPIC) += ompic.o +obj-$(CONFIG_ETRAXFS) += etraxfs_pic.o diff --git a/hw/intc/etraxfs_pic.c b/hw/intc/etraxfs_pic.c index 77f652acec..8065fc757e 100644 --- a/hw/intc/etraxfs_pic.c +++ b/hw/intc/etraxfs_pic.c @@ -27,8 +27,7 @@ #include "qemu/module.h" #include "hw/irq.h" #include "hw/qdev-properties.h" -//#include "pc.h" -//#include "etraxfs.h" +#include "target/cris/cpu.h" #define D(x) @@ -48,10 +47,10 @@ struct etrax_pic SysBusDevice parent_obj; MemoryRegion mmio; - void *interrupt_vector; qemu_irq parent_irq; qemu_irq parent_nmi; uint32_t regs[R_MAX]; + CRISCPU *cpu; }; static void pic_update(struct etrax_pic *fs) @@ -79,9 +78,10 @@ static void pic_update(struct etrax_pic *fs) } } - if (fs->interrupt_vector) { - /* hack alert: ptr property */ - *(uint32_t*)(fs->interrupt_vector) = vector; + if (fs->cpu) { + /* hack alert: cpu link property */ + int32_t *int_vec = &fs->cpu->env.interrupt_vector; + *int_vec = (uint32_t)vector; } qemu_set_irq(fs->parent_irq, !!vector); } @@ -164,7 +164,7 @@ static void etraxfs_pic_init(Object *obj) } static Property etraxfs_pic_properties[] = { - DEFINE_PROP_PTR("interrupt_vector", struct etrax_pic, interrupt_vector), + DEFINE_PROP_LINK("cpu", struct etrax_pic, cpu, TYPE_CRIS_CPU, CRISCPU *), DEFINE_PROP_END_OF_LIST(), }; @@ -173,10 +173,6 @@ static void etraxfs_pic_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc->props = etraxfs_pic_properties; - /* - * Note: pointer property "interrupt_vector" may remain null, thus - * no need for dc->user_creatable = false; - */ } static const TypeInfo etraxfs_pic_info = {
Instead of using raw interrupt vector pointer, store the associated CPU with a link property. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- hw/cris/axis_dev88.c | 4 +--- hw/intc/Makefile.objs | 2 +- hw/intc/etraxfs_pic.c | 18 +++++++----------- 3 files changed, 9 insertions(+), 15 deletions(-)