| Message ID | d05041e5213cebb2631ca2bfd4a54025ae577008.1470156587.git.alistair@alistair23.me (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 3 August 2016 at 04:10, Alistair Francis <alistair23@gmail.com> wrote: > Signed-off-by: Alistair Francis <alistair@alistair23.me> > --- > As the migration framework is not included in user mode this needs to be a > new file. > > V6: > - Make the OR IRQ device a TYPE_DEVICE > - Add vmstate > > hw/core/Makefile.objs | 1 + > hw/core/irq.c | 1 + > hw/core/or-irq.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/irq.h | 15 ++++++++ > 4 files changed, 119 insertions(+) > create mode 100644 hw/core/or-irq.c > > diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs > index cfd4840..b47241b 100644 > --- a/hw/core/Makefile.objs > +++ b/hw/core/Makefile.objs > @@ -16,4 +16,5 @@ common-obj-$(CONFIG_SOFTMMU) += null-machine.o > common-obj-$(CONFIG_SOFTMMU) += loader.o > common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o > common-obj-$(CONFIG_SOFTMMU) += register.o > +common-obj-$(CONFIG_SOFTMMU) += or-irq.o > common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o > diff --git a/hw/core/irq.c b/hw/core/irq.c > index 49ff2e6..dc874cc 100644 > --- a/hw/core/irq.c > +++ b/hw/core/irq.c > @@ -24,6 +24,7 @@ > #include "qemu/osdep.h" > #include "qemu-common.h" > #include "hw/irq.h" > +#include "hw/sysbus.h" > #include "qom/object.h" > > #define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ) We don't change anything else in this file so I think this added include is leftover from development? > diff --git a/hw/core/or-irq.c b/hw/core/or-irq.c > new file mode 100644 > index 0000000..2bd181b > --- /dev/null > +++ b/hw/core/or-irq.c > @@ -0,0 +1,102 @@ > +/* > + * QEMU IRQ/GPIO common code. > + * > + * Copyright (c) 2016 Alistair Francis <alistair@alistair23.me>. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > +#include "qemu/osdep.h" > +#include "qemu-common.h" > +#include "hw/irq.h" > +#include "hw/sysbus.h" > +#include "qom/object.h" > + > +#define OR_IRQ(obj) OBJECT_CHECK(qemu_or_irq, (obj), TYPE_OR_IRQ) > + > +struct OrIRQState { > + Object parent_obj; > + > + qemu_irq in_irq; > + qemu_irq *out_irqs; Aren't these names backwards? An OR gate has multiple inputs and one output. > + int16_t levels[MAX_OR_LINES]; Why is this an int16_t array? I was expecting an array of bool (or whatever the equivalent is for vmstate), or alternatively a bitmap.h bitmap with one bit per input. > + int n; > +}; > + > +static void or_irq_handler(void *opaque, int n, int level) > +{ > + qemu_or_irq *or_irq = OR_IRQ(opaque); > + int or_level = 0; > + int i; > + > + or_irq->levels[n] = level; > + > + for (i = 0; i < or_irq->n; i++) { > + or_level |= or_irq->levels[i]; > + } > + > + qemu_set_irq(or_irq->in_irq, or_level); > +} > + > +qemu_irq *qemu_allocate_or_irqs(qemu_irq in_irq, int n) This API is awkward because it means that there's no way for the caller to arrange for the OR_IRQ object to be freed (for instance if you want to use it in a hotpluggable device), unless I'm missing something. I was sort of expecting this to be just-another-device that you would wire up with the existing sysbus gpio APIs. > +{ > + qemu_or_irq *or_irq; > + > + assert(n < MAX_OR_LINES); > + > + or_irq = OR_IRQ(object_new(TYPE_OR_IRQ)); > + object_initialize(or_irq, sizeof(qemu_or_irq), > + TYPE_OR_IRQ); > + > + or_irq->out_irqs = qemu_allocate_irqs(or_irq_handler, or_irq, n); > + or_irq->in_irq = in_irq; > + or_irq->n = n; > + > + return or_irq->out_irqs; > +} > + > +static const VMStateDescription vmstate_or_irq = { > + .name = TYPE_OR_IRQ, > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_INT16_ARRAY(levels, qemu_or_irq, MAX_OR_LINES), > + VMSTATE_END_OF_LIST(), > + } > +}; > + > +static void or_irq_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->vmsd = &vmstate_or_irq; I think this also needs a reset method that just zeroes the levels array. > +} > + > +static const TypeInfo or_irq_type_info = { > + .name = TYPE_OR_IRQ, > + .parent = TYPE_DEVICE, > + .instance_size = sizeof(qemu_or_irq), > + .class_init = or_irq_class_init, > +}; > + > +static void or_irq_register_types(void) > +{ > + type_register_static(&or_irq_type_info); > +} > + > +type_init(or_irq_register_types) > diff --git a/include/hw/irq.h b/include/hw/irq.h > index 4c4c2ea..5e8a3b6 100644 > --- a/include/hw/irq.h > +++ b/include/hw/irq.h > @@ -4,8 +4,12 @@ > /* Generic IRQ/GPIO pin infrastructure. */ > > #define TYPE_IRQ "irq" > +#define TYPE_OR_IRQ "or-irq" > + > +#define MAX_OR_LINES 16 > > typedef struct IRQState *qemu_irq; > +typedef struct OrIRQState qemu_or_irq; > > typedef void (*qemu_irq_handler)(void *opaque, int n, int level); > > @@ -38,6 +42,17 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n); > */ > qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n); > > +/* > + * qemu_allocate_or_irqs > + * @in_irq: An input IRQ. It will be the result of the @out_irqs ORed together > + * @n: The number of interrupt lines that should be ORed together > + * > + * returns: An array of interrupts that should be ORed together > + * > + * OR all of the interrupts returned in the array into a single @in_irq. > + */ > +qemu_irq *qemu_allocate_or_irqs(qemu_irq in_irq, int n); > + > /* Extends an Array of IRQs. Old IRQs have their handlers and opaque data > * preserved. New IRQs are assigned the argument handler and opaque data. > */ > -- > 2.7.4 > thanks -- PMM
On Tue, Aug 9, 2016 at 9:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 3 August 2016 at 04:10, Alistair Francis <alistair23@gmail.com> wrote: >> Signed-off-by: Alistair Francis <alistair@alistair23.me> >> --- >> As the migration framework is not included in user mode this needs to be a >> new file. >> >> V6: >> - Make the OR IRQ device a TYPE_DEVICE >> - Add vmstate >> >> hw/core/Makefile.objs | 1 + >> hw/core/irq.c | 1 + >> hw/core/or-irq.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/irq.h | 15 ++++++++ >> 4 files changed, 119 insertions(+) >> create mode 100644 hw/core/or-irq.c >> >> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs >> index cfd4840..b47241b 100644 >> --- a/hw/core/Makefile.objs >> +++ b/hw/core/Makefile.objs >> @@ -16,4 +16,5 @@ common-obj-$(CONFIG_SOFTMMU) += null-machine.o >> common-obj-$(CONFIG_SOFTMMU) += loader.o >> common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o >> common-obj-$(CONFIG_SOFTMMU) += register.o >> +common-obj-$(CONFIG_SOFTMMU) += or-irq.o >> common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o >> diff --git a/hw/core/irq.c b/hw/core/irq.c >> index 49ff2e6..dc874cc 100644 >> --- a/hw/core/irq.c >> +++ b/hw/core/irq.c >> @@ -24,6 +24,7 @@ >> #include "qemu/osdep.h" >> #include "qemu-common.h" >> #include "hw/irq.h" >> +#include "hw/sysbus.h" >> #include "qom/object.h" >> >> #define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ) > > We don't change anything else in this file so I think this added include > is leftover from development? Yep, it is. I have fixed this. > >> diff --git a/hw/core/or-irq.c b/hw/core/or-irq.c >> new file mode 100644 >> index 0000000..2bd181b >> --- /dev/null >> +++ b/hw/core/or-irq.c >> @@ -0,0 +1,102 @@ >> +/* >> + * QEMU IRQ/GPIO common code. >> + * >> + * Copyright (c) 2016 Alistair Francis <alistair@alistair23.me>. >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a copy >> + * of this software and associated documentation files (the "Software"), to deal >> + * in the Software without restriction, including without limitation the rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell >> + * copies of the Software, and to permit persons to whom the Software is >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN >> + * THE SOFTWARE. >> + */ >> +#include "qemu/osdep.h" >> +#include "qemu-common.h" >> +#include "hw/irq.h" >> +#include "hw/sysbus.h" >> +#include "qom/object.h" >> + >> +#define OR_IRQ(obj) OBJECT_CHECK(qemu_or_irq, (obj), TYPE_OR_IRQ) >> + >> +struct OrIRQState { >> + Object parent_obj; >> + >> + qemu_irq in_irq; >> + qemu_irq *out_irqs; > > Aren't these names backwards? An OR gate has multiple inputs > and one output. I think I was thinking of the direction from the devices. So the out_irqs are the lines that go out from the device and the in_irq goes into the next device (the GIC in this case). Although that probably is somewhat confusing. > >> + int16_t levels[MAX_OR_LINES]; > > Why is this an int16_t array? I was expecting an array of bool > (or whatever the equivalent is for vmstate), or alternatively > a bitmap.h bitmap with one bit per input. It should be bools, I just couldn't find the VMstate macro for bools at the time. I have fixed this. > >> + int n; >> +}; >> + >> +static void or_irq_handler(void *opaque, int n, int level) >> +{ >> + qemu_or_irq *or_irq = OR_IRQ(opaque); >> + int or_level = 0; >> + int i; >> + >> + or_irq->levels[n] = level; >> + >> + for (i = 0; i < or_irq->n; i++) { >> + or_level |= or_irq->levels[i]; >> + } >> + >> + qemu_set_irq(or_irq->in_irq, or_level); >> +} >> + >> +qemu_irq *qemu_allocate_or_irqs(qemu_irq in_irq, int n) > > This API is awkward because it means that there's no way for > the caller to arrange for the OR_IRQ object to be freed > (for instance if you want to use it in a hotpluggable device), > unless I'm missing something. > > I was sort of expecting this to be just-another-device that > you would wire up with the existing sysbus gpio APIs. Ok, I have fixed this up. It now follows the standard API. > >> +{ >> + qemu_or_irq *or_irq; >> + >> + assert(n < MAX_OR_LINES); >> + >> + or_irq = OR_IRQ(object_new(TYPE_OR_IRQ)); >> + object_initialize(or_irq, sizeof(qemu_or_irq), >> + TYPE_OR_IRQ); >> + >> + or_irq->out_irqs = qemu_allocate_irqs(or_irq_handler, or_irq, n); >> + or_irq->in_irq = in_irq; >> + or_irq->n = n; >> + >> + return or_irq->out_irqs; >> +} >> + >> +static const VMStateDescription vmstate_or_irq = { >> + .name = TYPE_OR_IRQ, >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_INT16_ARRAY(levels, qemu_or_irq, MAX_OR_LINES), >> + VMSTATE_END_OF_LIST(), >> + } >> +}; >> + >> +static void or_irq_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + >> + dc->vmsd = &vmstate_or_irq; > > I think this also needs a reset method that just zeroes > the levels array. Fixed. Thanks, Alistair > >> +} >> + >> +static const TypeInfo or_irq_type_info = { >> + .name = TYPE_OR_IRQ, >> + .parent = TYPE_DEVICE, >> + .instance_size = sizeof(qemu_or_irq), >> + .class_init = or_irq_class_init, >> +}; >> + >> +static void or_irq_register_types(void) >> +{ >> + type_register_static(&or_irq_type_info); >> +} >> + >> +type_init(or_irq_register_types) >> diff --git a/include/hw/irq.h b/include/hw/irq.h >> index 4c4c2ea..5e8a3b6 100644 >> --- a/include/hw/irq.h >> +++ b/include/hw/irq.h >> @@ -4,8 +4,12 @@ >> /* Generic IRQ/GPIO pin infrastructure. */ >> >> #define TYPE_IRQ "irq" >> +#define TYPE_OR_IRQ "or-irq" >> + >> +#define MAX_OR_LINES 16 >> >> typedef struct IRQState *qemu_irq; >> +typedef struct OrIRQState qemu_or_irq; >> >> typedef void (*qemu_irq_handler)(void *opaque, int n, int level); >> >> @@ -38,6 +42,17 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n); >> */ >> qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n); >> >> +/* >> + * qemu_allocate_or_irqs >> + * @in_irq: An input IRQ. It will be the result of the @out_irqs ORed together >> + * @n: The number of interrupt lines that should be ORed together >> + * >> + * returns: An array of interrupts that should be ORed together >> + * >> + * OR all of the interrupts returned in the array into a single @in_irq. >> + */ >> +qemu_irq *qemu_allocate_or_irqs(qemu_irq in_irq, int n); >> + >> /* Extends an Array of IRQs. Old IRQs have their handlers and opaque data >> * preserved. New IRQs are assigned the argument handler and opaque data. >> */ >> -- >> 2.7.4 >> > > thanks > -- PMM
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs index cfd4840..b47241b 100644 --- a/hw/core/Makefile.objs +++ b/hw/core/Makefile.objs @@ -16,4 +16,5 @@ common-obj-$(CONFIG_SOFTMMU) += null-machine.o common-obj-$(CONFIG_SOFTMMU) += loader.o common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o common-obj-$(CONFIG_SOFTMMU) += register.o +common-obj-$(CONFIG_SOFTMMU) += or-irq.o common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o diff --git a/hw/core/irq.c b/hw/core/irq.c index 49ff2e6..dc874cc 100644 --- a/hw/core/irq.c +++ b/hw/core/irq.c @@ -24,6 +24,7 @@ #include "qemu/osdep.h" #include "qemu-common.h" #include "hw/irq.h" +#include "hw/sysbus.h" #include "qom/object.h" #define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ) diff --git a/hw/core/or-irq.c b/hw/core/or-irq.c new file mode 100644 index 0000000..2bd181b --- /dev/null +++ b/hw/core/or-irq.c @@ -0,0 +1,102 @@ +/* + * QEMU IRQ/GPIO common code. + * + * Copyright (c) 2016 Alistair Francis <alistair@alistair23.me>. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "hw/irq.h" +#include "hw/sysbus.h" +#include "qom/object.h" + +#define OR_IRQ(obj) OBJECT_CHECK(qemu_or_irq, (obj), TYPE_OR_IRQ) + +struct OrIRQState { + Object parent_obj; + + qemu_irq in_irq; + qemu_irq *out_irqs; + int16_t levels[MAX_OR_LINES]; + int n; +}; + +static void or_irq_handler(void *opaque, int n, int level) +{ + qemu_or_irq *or_irq = OR_IRQ(opaque); + int or_level = 0; + int i; + + or_irq->levels[n] = level; + + for (i = 0; i < or_irq->n; i++) { + or_level |= or_irq->levels[i]; + } + + qemu_set_irq(or_irq->in_irq, or_level); +} + +qemu_irq *qemu_allocate_or_irqs(qemu_irq in_irq, int n) +{ + qemu_or_irq *or_irq; + + assert(n < MAX_OR_LINES); + + or_irq = OR_IRQ(object_new(TYPE_OR_IRQ)); + object_initialize(or_irq, sizeof(qemu_or_irq), + TYPE_OR_IRQ); + + or_irq->out_irqs = qemu_allocate_irqs(or_irq_handler, or_irq, n); + or_irq->in_irq = in_irq; + or_irq->n = n; + + return or_irq->out_irqs; +} + +static const VMStateDescription vmstate_or_irq = { + .name = TYPE_OR_IRQ, + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_INT16_ARRAY(levels, qemu_or_irq, MAX_OR_LINES), + VMSTATE_END_OF_LIST(), + } +}; + +static void or_irq_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->vmsd = &vmstate_or_irq; +} + +static const TypeInfo or_irq_type_info = { + .name = TYPE_OR_IRQ, + .parent = TYPE_DEVICE, + .instance_size = sizeof(qemu_or_irq), + .class_init = or_irq_class_init, +}; + +static void or_irq_register_types(void) +{ + type_register_static(&or_irq_type_info); +} + +type_init(or_irq_register_types) diff --git a/include/hw/irq.h b/include/hw/irq.h index 4c4c2ea..5e8a3b6 100644 --- a/include/hw/irq.h +++ b/include/hw/irq.h @@ -4,8 +4,12 @@ /* Generic IRQ/GPIO pin infrastructure. */ #define TYPE_IRQ "irq" +#define TYPE_OR_IRQ "or-irq" + +#define MAX_OR_LINES 16 typedef struct IRQState *qemu_irq; +typedef struct OrIRQState qemu_or_irq; typedef void (*qemu_irq_handler)(void *opaque, int n, int level); @@ -38,6 +42,17 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n); */ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n); +/* + * qemu_allocate_or_irqs + * @in_irq: An input IRQ. It will be the result of the @out_irqs ORed together + * @n: The number of interrupt lines that should be ORed together + * + * returns: An array of interrupts that should be ORed together + * + * OR all of the interrupts returned in the array into a single @in_irq. + */ +qemu_irq *qemu_allocate_or_irqs(qemu_irq in_irq, int n); + /* Extends an Array of IRQs. Old IRQs have their handlers and opaque data * preserved. New IRQs are assigned the argument handler and opaque data. */
Signed-off-by: Alistair Francis <alistair@alistair23.me> --- As the migration framework is not included in user mode this needs to be a new file. V6: - Make the OR IRQ device a TYPE_DEVICE - Add vmstate hw/core/Makefile.objs | 1 + hw/core/irq.c | 1 + hw/core/or-irq.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/hw/irq.h | 15 ++++++++ 4 files changed, 119 insertions(+) create mode 100644 hw/core/or-irq.c