The handling of the GLUE (General Logic Unit) device is
currently open-coded. Make this into a proper QOM device.
This minor piece of modernisation gets rid of the free
floating qemu_irq array 'pic', which Coverity points out
is technically leaked when we exit the machine init function.
(The replacement glue device is not leaked because it gets
added to the sysbus, so it's accessible via that.)
Fixes: Coverity CID 1421883
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/m68k/q800.c | 82 ++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 12 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index dc13007aaf2..05bb372f958 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -47,6 +47,7 @@
#include "sysemu/qtest.h"
#include "sysemu/runstate.h"
#include "sysemu/reset.h"
+#include "migration/vmstate.h"
#define MACROM_ADDR 0x40800000
#define MACROM_SIZE 0x00100000
@@ -94,10 +95,14 @@
* CPU.
*/
-typedef struct {
+#define TYPE_GLUE "q800-glue"
+OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
+
+struct GLUEState {
+ SysBusDevice parent_obj;
M68kCPU *cpu;
uint8_t ipr;
-} GLUEState;
+};
static void GLUE_set_irq(void *opaque, int irq, int level)
{
@@ -119,6 +124,58 @@ static void GLUE_set_irq(void *opaque, int irq, int level)
m68k_set_irq_level(s->cpu, 0, 0);
}
+static void glue_reset(DeviceState *dev)
+{
+ GLUEState *s = GLUE(dev);
+
+ s->ipr = 0;
+}
+
+static const VMStateDescription vmstate_glue = {
+ .name = "q800-glue",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT8(ipr, GLUEState),
+ VMSTATE_END_OF_LIST(),
+ },
+};
+
+/*
+ * If the m68k CPU implemented its inbound irq lines as GPIO lines
+ * rather than via the m68k_set_irq_level() function we would not need
+ * this cpu link property and could instead provide outbound IRQ lines
+ * that the board could wire up to the CPU.
+ */
+static Property glue_properties[] = {
+ DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void glue_init(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ qdev_init_gpio_in(dev, GLUE_set_irq, 8);
+}
+
+static void glue_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->vmsd = &vmstate_glue;
+ dc->reset = glue_reset;
+ device_class_set_props(dc, glue_properties);
+}
+
+static const TypeInfo glue_info = {
+ .name = TYPE_GLUE,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(GLUEState),
+ .instance_init = glue_init,
+ .class_init = glue_class_init,
+};
+
static void main_cpu_reset(void *opaque)
{
M68kCPU *cpu = opaque;
@@ -178,8 +235,7 @@ static void q800_init(MachineState *machine)
SysBusDevice *sysbus;
BusState *adb_bus;
NubusBus *nubus;
- GLUEState *irq;
- qemu_irq *pic;
+ DeviceState *glue;
DriveInfo *dinfo;
linux_boot = (kernel_filename != NULL);
@@ -213,10 +269,9 @@ static void q800_init(MachineState *machine)
}
/* IRQ Glue */
-
- irq = g_new0(GLUEState, 1);
- irq->cpu = cpu;
- pic = qemu_allocate_irqs(GLUE_set_irq, irq, 8);
+ glue = qdev_new(TYPE_GLUE);
+ object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
/* VIA */
@@ -228,8 +283,10 @@ static void q800_init(MachineState *machine)
sysbus = SYS_BUS_DEVICE(via_dev);
sysbus_realize_and_unref(sysbus, &error_fatal);
sysbus_mmio_map(sysbus, 0, VIA_BASE);
- qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0, pic[0]);
- qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 1, pic[1]);
+ qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0,
+ qdev_get_gpio_in(glue, 0));
+ qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 1,
+ qdev_get_gpio_in(glue, 1));
adb_bus = qdev_get_child_bus(via_dev, "adb.0");
@@ -270,7 +327,7 @@ static void q800_init(MachineState *machine)
sysbus_realize_and_unref(sysbus, &error_fatal);
sysbus_mmio_map(sysbus, 0, SONIC_BASE);
sysbus_mmio_map(sysbus, 1, SONIC_PROM_BASE);
- sysbus_connect_irq(sysbus, 0, pic[2]);
+ sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(glue, 2));
/* SCC */
@@ -292,7 +349,7 @@ static void q800_init(MachineState *machine)
qdev_realize_and_unref(escc_orgate, NULL, &error_fatal);
sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(escc_orgate, 0));
sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(escc_orgate, 1));
- qdev_connect_gpio_out(DEVICE(escc_orgate), 0, pic[3]);
+ qdev_connect_gpio_out(DEVICE(escc_orgate), 0, qdev_get_gpio_in(glue, 3));
sysbus_mmio_map(sysbus, 0, SCC_BASE);
/* SCSI */
@@ -457,6 +514,7 @@ static const TypeInfo q800_machine_typeinfo = {
static void q800_machine_register_types(void)
{
type_register_static(&q800_machine_typeinfo);
+ type_register_static(&glue_info);
}
type_init(q800_machine_register_types)
--
2.20.1
Hi Peter,
On 11/7/20 12:51 AM, Peter Maydell wrote:
> The handling of the GLUE (General Logic Unit) device is
> currently open-coded. Make this into a proper QOM device.
>
> This minor piece of modernisation gets rid of the free
> floating qemu_irq array 'pic', which Coverity points out
> is technically leaked when we exit the machine init function.
> (The replacement glue device is not leaked because it gets
> added to the sysbus, so it's accessible via that.)
>
> Fixes: Coverity CID 1421883
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/m68k/q800.c | 82 ++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 70 insertions(+), 12 deletions(-)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index dc13007aaf2..05bb372f958 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -47,6 +47,7 @@
> #include "sysemu/qtest.h"
> #include "sysemu/runstate.h"
> #include "sysemu/reset.h"
> +#include "migration/vmstate.h"
>
> #define MACROM_ADDR 0x40800000
> #define MACROM_SIZE 0x00100000
> @@ -94,10 +95,14 @@
> * CPU.
> */
>
> -typedef struct {
> +#define TYPE_GLUE "q800-glue"
> +OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
> +
> +struct GLUEState {
> + SysBusDevice parent_obj;
> M68kCPU *cpu;
> uint8_t ipr;
> -} GLUEState;
> +};
>
> static void GLUE_set_irq(void *opaque, int irq, int level)
> {
> @@ -119,6 +124,58 @@ static void GLUE_set_irq(void *opaque, int irq, int level)
> m68k_set_irq_level(s->cpu, 0, 0);
> }
>
> +static void glue_reset(DeviceState *dev)
> +{
> + GLUEState *s = GLUE(dev);
> +
> + s->ipr = 0;
> +}
> +
> +static const VMStateDescription vmstate_glue = {
> + .name = "q800-glue",
> + .version_id = 0,
> + .minimum_version_id = 0,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT8(ipr, GLUEState),
> + VMSTATE_END_OF_LIST(),
> + },
> +};
> +
> +/*
> + * If the m68k CPU implemented its inbound irq lines as GPIO lines
> + * rather than via the m68k_set_irq_level() function we would not need
> + * this cpu link property and could instead provide outbound IRQ lines
> + * that the board could wire up to the CPU.
> + */
> +static Property glue_properties[] = {
> + DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void glue_init(Object *obj)
> +{
> + DeviceState *dev = DEVICE(obj);
> +
> + qdev_init_gpio_in(dev, GLUE_set_irq, 8);
> +}
> +
> +static void glue_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->vmsd = &vmstate_glue;
> + dc->reset = glue_reset;
Don't we need a realize() handler checking s->cpu is non-NULL?
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> + device_class_set_props(dc, glue_properties);
> +}
> +
> +static const TypeInfo glue_info = {
> + .name = TYPE_GLUE,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(GLUEState),
> + .instance_init = glue_init,
> + .class_init = glue_class_init,
> +};
> +
> static void main_cpu_reset(void *opaque)
> {
> M68kCPU *cpu = opaque;
> @@ -178,8 +235,7 @@ static void q800_init(MachineState *machine)
> SysBusDevice *sysbus;
> BusState *adb_bus;
> NubusBus *nubus;
> - GLUEState *irq;
> - qemu_irq *pic;
> + DeviceState *glue;
> DriveInfo *dinfo;
>
> linux_boot = (kernel_filename != NULL);
> @@ -213,10 +269,9 @@ static void q800_init(MachineState *machine)
> }
>
> /* IRQ Glue */
> -
> - irq = g_new0(GLUEState, 1);
> - irq->cpu = cpu;
> - pic = qemu_allocate_irqs(GLUE_set_irq, irq, 8);
> + glue = qdev_new(TYPE_GLUE);
> + object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
>
> /* VIA */
>
> @@ -228,8 +283,10 @@ static void q800_init(MachineState *machine)
> sysbus = SYS_BUS_DEVICE(via_dev);
> sysbus_realize_and_unref(sysbus, &error_fatal);
> sysbus_mmio_map(sysbus, 0, VIA_BASE);
> - qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0, pic[0]);
> - qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 1, pic[1]);
> + qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0,
> + qdev_get_gpio_in(glue, 0));
> + qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 1,
> + qdev_get_gpio_in(glue, 1));
>
>
> adb_bus = qdev_get_child_bus(via_dev, "adb.0");
> @@ -270,7 +327,7 @@ static void q800_init(MachineState *machine)
> sysbus_realize_and_unref(sysbus, &error_fatal);
> sysbus_mmio_map(sysbus, 0, SONIC_BASE);
> sysbus_mmio_map(sysbus, 1, SONIC_PROM_BASE);
> - sysbus_connect_irq(sysbus, 0, pic[2]);
> + sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(glue, 2));
>
> /* SCC */
>
> @@ -292,7 +349,7 @@ static void q800_init(MachineState *machine)
> qdev_realize_and_unref(escc_orgate, NULL, &error_fatal);
> sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(escc_orgate, 0));
> sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(escc_orgate, 1));
> - qdev_connect_gpio_out(DEVICE(escc_orgate), 0, pic[3]);
> + qdev_connect_gpio_out(DEVICE(escc_orgate), 0, qdev_get_gpio_in(glue, 3));
> sysbus_mmio_map(sysbus, 0, SCC_BASE);
>
> /* SCSI */
> @@ -457,6 +514,7 @@ static const TypeInfo q800_machine_typeinfo = {
> static void q800_machine_register_types(void)
> {
> type_register_static(&q800_machine_typeinfo);
> + type_register_static(&glue_info);
> }
>
> type_init(q800_machine_register_types)
>
On Mon, 9 Nov 2020 at 14:15, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Peter,
>
> On 11/7/20 12:51 AM, Peter Maydell wrote:
> > The handling of the GLUE (General Logic Unit) device is
> > currently open-coded. Make this into a proper QOM device.
> >
> > This minor piece of modernisation gets rid of the free
> > floating qemu_irq array 'pic', which Coverity points out
> > is technically leaked when we exit the machine init function.
> > (The replacement glue device is not leaked because it gets
> > added to the sysbus, so it's accessible via that.)
> >
> > Fixes: Coverity CID 1421883
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > hw/m68k/q800.c | 82 ++++++++++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 70 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> > index dc13007aaf2..05bb372f958 100644
> > --- a/hw/m68k/q800.c
> > +++ b/hw/m68k/q800.c
> > @@ -47,6 +47,7 @@
> > #include "sysemu/qtest.h"
> > #include "sysemu/runstate.h"
> > #include "sysemu/reset.h"
> > +#include "migration/vmstate.h"
> >
> > #define MACROM_ADDR 0x40800000
> > #define MACROM_SIZE 0x00100000
> > @@ -94,10 +95,14 @@
> > * CPU.
> > */
> >
> > -typedef struct {
> > +#define TYPE_GLUE "q800-glue"
> > +OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
> > +
> > +struct GLUEState {
> > + SysBusDevice parent_obj;
> > M68kCPU *cpu;
> > uint8_t ipr;
> > -} GLUEState;
> > +};
> >
> > static void GLUE_set_irq(void *opaque, int irq, int level)
> > {
> > @@ -119,6 +124,58 @@ static void GLUE_set_irq(void *opaque, int irq, int level)
> > m68k_set_irq_level(s->cpu, 0, 0);
> > }
> >
> > +static void glue_reset(DeviceState *dev)
> > +{
> > + GLUEState *s = GLUE(dev);
> > +
> > + s->ipr = 0;
> > +}
> > +
> > +static const VMStateDescription vmstate_glue = {
> > + .name = "q800-glue",
> > + .version_id = 0,
> > + .minimum_version_id = 0,
> > + .fields = (VMStateField[]) {
> > + VMSTATE_UINT8(ipr, GLUEState),
> > + VMSTATE_END_OF_LIST(),
> > + },
> > +};
> > +
> > +/*
> > + * If the m68k CPU implemented its inbound irq lines as GPIO lines
> > + * rather than via the m68k_set_irq_level() function we would not need
> > + * this cpu link property and could instead provide outbound IRQ lines
> > + * that the board could wire up to the CPU.
> > + */
> > +static Property glue_properties[] = {
> > + DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
> > + DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void glue_init(Object *obj)
> > +{
> > + DeviceState *dev = DEVICE(obj);
> > +
> > + qdev_init_gpio_in(dev, GLUE_set_irq, 8);
> > +}
> > +
> > +static void glue_class_init(ObjectClass *klass, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > + dc->vmsd = &vmstate_glue;
> > + dc->reset = glue_reset;
>
> Don't we need a realize() handler checking s->cpu is non-NULL?
>
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Doesn't seem very necessary to me -- it's a sysbus device
used for a special purpose, the one user has to wire
it up correctly, and the failure mode if they get it wrong
is pretty obvious. But I guess we could add in the explicit
error check.
thanks
-- PMM
Le 07/11/2020 à 00:51, Peter Maydell a écrit : > The handling of the GLUE (General Logic Unit) device is > currently open-coded. Make this into a proper QOM device. > > This minor piece of modernisation gets rid of the free > floating qemu_irq array 'pic', which Coverity points out > is technically leaked when we exit the machine init function. > (The replacement glue device is not leaked because it gets > added to the sysbus, so it's accessible via that.) > > Fixes: Coverity CID 1421883 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/m68k/q800.c | 82 ++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 70 insertions(+), 12 deletions(-) > Reviewed-by: Laurent vivier <laurent@vivier.eu>
© 2016 - 2026 Red Hat, Inc.